Skip to content

Skip test_lazy when html.parser is already imported#405

Open
l46983284-cpu wants to merge 4 commits into
benjaminp:mainfrom
l46983284-cpu:fix-335-skip-test-lazy-if-html-preloaded
Open

Skip test_lazy when html.parser is already imported#405
l46983284-cpu wants to merge 4 commits into
benjaminp:mainfrom
l46983284-cpu:fix-335-skip-test-lazy-if-html-preloaded

Conversation

@l46983284-cpu

Copy link
Copy Markdown

Summary

test_lazy asserts that html.parser / HTMLParser is not already imported so it can prove lazy loading via six.moves.html_parser. Environments that import Django (or otherwise preload html.parser) fail that precondition.

This skips the test when the module is already present, matching the existing optional-module skip style in test_six.py.

Fixes #335

Test plan

  • clean: pytest test_six.py::test_lazy → pass
  • preloaded html.parser: same test → skip
  • full: pytest test_six.py → 198 passed, 2 skipped

Django and similar packages import html.parser at startup, which makes
the lazy-load pre-condition of test_lazy impossible. Skip in that case
instead of failing the suite.

Fixes benjaminp#335

Signed-off-by: Alex Chen <l46983284@gmail.com>
@benjaminp

Copy link
Copy Markdown
Owner

Why would Django be imported in the six test suite?

The six suite does not import Django. Skip only when the lazy-load
pre-condition is already broken because html.parser/HTMLParser is
present in sys.modules from the surrounding environment.

Signed-off-by: Alex Chen <l46983284@gmail.com>
@l46983284-cpu

Copy link
Copy Markdown
Author

It wouldn't. six doesn't import Django.

Issue #335 is about running the suite in an environment where Django (or anything else) has already imported html.parser, so assert html_name not in sys.modules fails before the lazy-load check can run. I dropped the Django wording from the skip; it now only skips when that pre-condition is already broken.

@benjaminp

Copy link
Copy Markdown
Owner

Why does django get loaded into six's test process though?

six does not import Django. When Django is installed and an ambient
pytest plugin imports it at collection time (hypothesis.internal.compat
-> django.test -> django.utils.html), html.parser is already in
sys.modules and the lazy-load pre-condition cannot hold. Skip in that
case; keep the test when the process is clean.

Signed-off-by: Alex Chen <l46983284@gmail.com>
@l46983284-cpu

Copy link
Copy Markdown
Author

It is not six that loads Django.

On issue #335 the pytest banner shows ambient plugins, including hypothesis. When Django is also installed in that environment, importing the hypothesis pytest plugin pulls hypothesis.internal.compat, which does from django.test import TransactionTestCase if Django is importable. That import graph reaches django.utils.html, which imports stdlib html.parser before test_lazy runs.

Controls I ran:

  • Django installed, no hypothesis plugin: import django does not load html.parser; test_lazy passes.
  • Django + hypothesis plugin entrypoint load: both django and html.parser are already in sys.modules at collection time.
  • Direct import django.utils.html is enough to put html.parser in sys.modules.

So the failure mode is polluted site-packages / plugin autoload, not the six test suite importing Django. The skip is only for the broken pre-condition (html.parser already imported). I expanded the comment in the PR to spell that path out.

@benjaminp

Copy link
Copy Markdown
Owner

I'm not sure we can be robust about global pollution of the Python environment. I wonder if there's a way to avoid picking up arbitrary and unneeded plugins like this.

test_lazy needs a clean process. Ambient site-packages plugins can be
auto-loaded by pytest and import unrelated packages before the suite
runs. Set PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 in tox (pytest-only deps) so
the suite does not pick up those plugins. Restore the original
pre-condition assert instead of skipping on pollution.

Signed-off-by: Alex Chen <l46983284@gmail.com>
@l46983284-cpu

Copy link
Copy Markdown
Author

Good point — the failure comes from pytest auto-loading installed plugins, not from six importing anything extra.

I changed the approach: PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 in tox.ini so the suite only uses pytest itself (tox already depends on pytest alone). That env var has been supported for a long time; older pytest just ignores it, so it stays safe for the old matrix. I did not put --disable-plugin-autoload in setup.cfg/addopts, because that flag is new (8.4+) and would break older pytest.

I also dropped the skip-based workaround and restored the original assert html_name not in sys.modules pre-condition under that cleaner invocation.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test_lazy fails if django is installed

2 participants