Skip to content

tests: keep environment from leaking in Python 3.15#1049

Merged
henryiii merged 4 commits intomainfrom
henryiii/fix/315leak
Apr 22, 2026
Merged

tests: keep environment from leaking in Python 3.15#1049
henryiii merged 4 commits intomainfrom
henryiii/fix/315leak

Conversation

@henryiii
Copy link
Copy Markdown
Contributor

@henryiii henryiii commented Apr 21, 2026

Description

Fix #1047.

🤖 Use a bit of copilot with both gpt-5-mini and Kimi-K2.6, mostly to automate running and appying the example. Hand wrote most of the config changes and some of the code changes.

Changelog

  • Added changelog fragment: docs/changelog/<pr_number>.<type>.rst
    • Types: feature, bugfix, doc, removal, misc
    • Example: 123.feature.rst containing Add custom backend support - by :user:`yourname`

Checklist

  • Tests pass locally (tox)
  • Code follows project style (tox -e fix)
  • Type checks pass (tox -e type)
  • Documentation builds (tox -e docs)

@henryiii henryiii marked this pull request as draft April 21, 2026 20:37
@henryiii henryiii marked this pull request as draft April 21, 2026 20:37
@henryiii henryiii force-pushed the henryiii/fix/315leak branch from ff04f6d to 5e4e16e Compare April 21, 2026 20:39
@henryiii henryiii changed the title fix: keep environemnt from leaking in Python 3.15 tests: keep environemnt from leaking in Python 3.15 Apr 21, 2026
@henryiii henryiii changed the title tests: keep environemnt from leaking in Python 3.15 tests: keep environment from leaking in Python 3.15 Apr 21, 2026
Signed-off-by: Henry Schreiner <henryfs@princeton.edu>
@henryiii henryiii force-pushed the henryiii/fix/315leak branch from 5e4e16e to 9ab2ac0 Compare April 21, 2026 20:47
@henryiii
Copy link
Copy Markdown
Contributor Author

@copilot investigate the CI failure.

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 21, 2026

@copilot investigate the CI failure.

The CI failure was a coverage failure: src/build/_types.py had 0% coverage on Python 3.15.

Root cause: Python 3.15 uses __lazy_modules__ as a native lazy imports mechanism. Since _types is listed in __lazy_modules__ in build/__init__.py, the from ._types import ConfigSettings as ConfigSettingsType lines are deferred until those names are actually accessed. Since no tests access build.ConfigSettingsType, build.DistributionType, or build.RunnerType, _types.py was never executed — resulting in 0% coverage and failing the fail-under=100 check.

Fix (commit 6990945): Added src/build/_types.py to the coverage run.omit list. This is appropriate since _types.py only contains type aliases (no logic to test).

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • docs.python.org
    • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node /home/REDACTED/work/_temp/ghcca-node/node/bin/node --enable-source-maps /home/REDACTED/work/_temp/copilot-developer-action-main/dist/index.js (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@layday
Copy link
Copy Markdown
Member

layday commented Apr 21, 2026

I feel both of these tests are too generic to be meaningful. What are we testing, that a venv is a venv? env.python_executable != sys.executable is probably sufficient.

@henryiii
Copy link
Copy Markdown
Contributor Author

I can probably combine the tests. The first one is the original one, with the added protection from #1047. The second is the same test but with an injected PYTHONPATH to verify it fails regardless of the external PYTHONPATH setting - but I think we could just add that to the first test.

Signed-off-by: Henry Schreiner <henryfs@princeton.edu>
@henryiii henryiii force-pushed the henryiii/fix/315leak branch from 053b394 to 36db83a Compare April 21, 2026 22:45
Signed-off-by: Henry Schreiner <henryfs@princeton.edu>
@henryiii
Copy link
Copy Markdown
Contributor Author

I believe this issue wouldn't show up if we just checked env.python_executable != sys.executable. This demonstrates that you need to apply the extra env. If you remove it, the test fails.

@henryiii henryiii marked this pull request as ready for review April 22, 2026 16:12
@layday
Copy link
Copy Markdown
Member

layday commented Apr 22, 2026

The issue only exists in the test. Whether we guard against PYTHONPATH in this one test or not has no impact on the actual running of the tool, so I don’t think it matters much either way.

@henryiii
Copy link
Copy Markdown
Contributor Author

henryiii commented Apr 22, 2026

It shows that we need to guard it in the code itself (yes, this is way too unit-testy of a unit-test, testing the internals by mimicking the internals instead of using them!)

If we want to drop the test or change it, I think that should be a followup. This only fixes the current test to work on 3.15, and adds 3.15 to our CI.

@henryiii henryiii merged commit 373b9ee into main Apr 22, 2026
71 checks passed
@henryiii henryiii deleted the henryiii/fix/315leak branch April 22, 2026 20:05
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_isolation fails when PYTHONPATH is set (exposed by __lazy_modules__ on Python 3.15)

3 participants