Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a network pytest mark for tests that use the network #982

Closed
mcepl opened this issue Jan 5, 2022 · 4 comments · Fixed by #983
Closed

Add a network pytest mark for tests that use the network #982

mcepl opened this issue Jan 5, 2022 · 4 comments · Fixed by #983

Comments

@mcepl
Copy link

mcepl commented Jan 5, 2022

Sometimes it's useful to have the tests that use the network marked so they can be skipped easily when we know the network is not available.

This is useful for example on SUSE and openSUSE's build servers. When building our packages the network is disabled so we can assure reproducible builds (among other benefits). With this mark, it's easier to skip tests that can not succeed.

Patch suggesting the solution.

Complete build logs with all packages used and steps taken to reproduce.

@dairiki
Copy link
Contributor

dairiki commented Jan 6, 2022

This seems like a good idea.

I see you have two tests marked as requiring a network connection.

I think that only test_path_installed_plugin_is_none is failing because of missing internet connectivity. (It tries to install the lektor-webpack-support from PyPI.)

The second test you marked, test_markdown_links appears to be failing for you due to improper encoding of ; to %3B in a data: URL. (At least, I'm pretty sure that semicolon should not be encoded. See RFC 2397.) Anyway, that failure has nothing to do with lack of internet.

From your _log.txt (line 3279):

[    9s] _____________________________ test_markdown_links ______________________________
[    9s] 
[    9s] env = <lektor.environment.Environment object at 0x7f50ff0be400>
[    9s] pad = <lektor.db.Pad object at 0x7f50fef50670>
[    9s] 
[    9s]     def test_markdown_links(env, pad):
[    9s]         field = make_field(env, "markdown")
[    9s]         source = DummySource()
[    9s]     
[    9s]         def md(s):
[    9s]             rv = field.deserialize_value(s, pad=pad)
[    9s]             assert isinstance(rv, MarkdownDescriptor)
[    9s]             return str(rv.__get__(source)).strip()
[    9s]     
[    9s]         with Context(pad=pad):
[    9s]             assert md("[foo](http://example.com/)") == (
[    9s]                 '<p><a href="http://example.com/">foo</a></p>'
[    9s]             )
[    9s]             assert md("[foo](javascript:foo)") == (
[    9s]                 '<p><a href="javascript:foo">foo</a></p>'
[    9s]             )
[    9s]     
[    9s]             img = (
[    9s]                 "iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4"
[    9s]                 "//8/w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg=="
[    9s]             )
[    9s] >           assert (
[    9s]                 md("![test](data:image/png;base64,%s)" % img)
[    9s]                 == ('<p><img src="data:image/png;base64,%s" alt="test"></p>') % img
[    9s]             )
[    9s] E           assert '<p><img src=...t="test"></p>' == '<p><img src=...t="test"></p>'
[    9s] E             - <p><img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg==" alt="test"></p>
[    9s] E             ?                            ^
[    9s] E             + <p><img src="data:image/png%3Bbase64,iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg==" alt="test"></p>
[    9s] E             ?                            ^^^
[    9s] 
[    9s] tests/test_types.py:67: AssertionError

I think this might be due to your use of mistune 2.0.1. (Lektor currently pins mistune<2.) Perhaps a buglet in mistune v2? I haven't looked closely enough to say for sure.

As a quick test, I just unplugged my ethernet cable and ran the tests. tests/test_pluginsystem.py::TestPlugin::test_path_installed_plugin_is_none is the only test that failed.

@mcepl
Copy link
Author

mcepl commented Jan 6, 2022

The second test you marked, test_markdown_links appears to be failing for you due to improper encoding of ; to %3B in a data: URL. (At least, I'm pretty sure that semicolon should not be encoded. See RFC 2397.) Anyway, that failure has nothing to do with lack of internet.

I think this might be due to your use of mistune 2.0.1. (Lektor currently pins mistune<2.) Perhaps a buglet in mistune v2? I haven't looked closely enough to say for sure.

I use #944 and it seems to work (except of this test apparently). Do you know where the problem is? field.deserialize_value() is lektor’s method, isn’t it?

bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this issue Jan 6, 2022
https://build.opensuse.org/request/show/944195
by user mcepl + dimstar_suse
- Upgrade to 3.3.0:
  - This release drops support for versions of Python before
    3.6. In particular, Python 2.7 is no longer supported.
  - Quite a few bugs have been fixed since the previous release.
  - The Admin UI has seen a major refactor and various
    performance optimisations. It has been rewritten in
    Typescript, and updated to use v5 of the Bootstrap CSS
    framework.
- Remove unnecessary patches:
  - more_recent_werkzeug.patch
  - werkzeug_rename.patch
- Added patches making things working:
  - new_version_of_mistune.patch (gh#lektor/lektor#944) make the
    package work with mistune 2.*
  - skip-network-tests.patch (gh#lektor/lektor#982) we build in
    the network isolated environment, we need to make tests which
    need it
@dairiki
Copy link
Contributor

dairiki commented Jan 6, 2022

I use #944 and it seems to work (except of this test apparently). Do you know where the problem is? field.deserialize_value() is lektor’s method, isn’t it?

@mcepl I believe the problem is with mistune==2.0.1. (The basic issue — that of overly aggressive %-escaping of URLs — exists in mistune==2.0.0 as well, but due to an oversight where image src URLs were not being escaped at all, I don't think it triggers this particular test failure in our test suite.)

I've just filed PR lepture/mistune#295 which, I think, fixes the issue.

@mcepl
Copy link
Author

mcepl commented Jan 6, 2022

I've just filed PR lepture/mistune#295 which, I think, fixes the issue.

Yes, that fixes it. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants