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

fix skip with package = wheel #3269

Merged
merged 10 commits into from Apr 27, 2024
Merged

Conversation

MarcinKonowalczyk
Copy link
Contributor

@MarcinKonowalczyk MarcinKonowalczyk commented Apr 21, 2024

Fix for #3153

  • ran the linter to address style issues (tox -e fix)
  • wrote descriptive pull request text
  • ensured there are test(s) validating the fix
  • added news fragment in docs/changelog folder
  • [n/a] updated/extended the documentation

Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test please

@gaborbernat gaborbernat marked this pull request as draft April 23, 2024 16:43
@MarcinKonowalczyk
Copy link
Contributor Author

Ok, the only failing test is test_depends.py due to missing output lines:
Screenshot 2024-04-26 at 18 28 13

Any hint as to where am I supposed to return that status? (sorry, a bit lost in the code) so far i'm looking at report called from execute, both in common.py

@MarcinKonowalczyk
Copy link
Contributor Author

For reference, here is the stack from the failing case:

Traceback (most recent call last):
  File ".../tox/src/tox/run.py", line 20, in run
    result = main(sys.argv[1:] if args is None else args)
  File ".../tox/src/tox/run.py", line 46, in main
    return handler(state)
  File ".../tox/src/tox/session/cmd/legacy.py", line 115, in legacy
    return run_sequential(state)
  File ".../tox/src/tox/session/cmd/run/sequential.py", line 25, in run_sequential
    return execute(state, max_workers=1, has_spinner=False, live=True)
  File ".../tox/src/tox/session/cmd/run/common.py", line 170, in execute
    cast(RunToxEnv, state.envs[name]).mark_active()
  File ".../tox/src/tox/tox_env/runner.py", line 211, in mark_active
    for pkg_env in self.package_envs:
  File ".../tox/src/tox/tox_env/runner.py", line 208, in package_envs
    yield from self.package_env.child_pkg_envs(self.conf)
  File ".../tox/src/tox/tox_env/python/package.py", line 124, in child_pkg_envs
    conf = run_conf["wheel_build_env"]
  File ".../tox/src/tox/config/sets.py", line 116, in __getitem__
    return self.load(item)
  File ".../tox/src/tox/config/sets.py", line 127, in load
    return config_definition.__call__(self._conf, self.loaders, ConfigLoadArgs(chain, self.name, self.env_name))  # noqa: PLC2801
  File ".../tox/src/tox/config/of_type.py", line 111, in __call__
    value = self.default(conf, args.env_name) if callable(self.default) else self.default
  File ".../tox/src/tox/tox_env/python/package.py", line 93, in default_wheel_tag
    run_py = cast(Python, run_env).base_python
  File ".../tox/src/tox/tox_env/python/api.py", line 260, in base_python
    raise Skip(msg)
tox.tox_env.errors.Skip: could not find python interpreter with spec(s): py310

@MarcinKonowalczyk
Copy link
Contributor Author

ok, compared and matched the behaviour to package = sdist. After stepping through the code it looks like it was nothing to do with the report, which looks correct when running the normal tox command with the patch on a test repo.

@MarcinKonowalczyk
Copy link
Contributor Author

MarcinKonowalczyk commented Apr 27, 2024

Also patched _make_tox_wheel to allow pytest --run-integration run with a dev patch version, to allow all tests to pass locally.

@MarcinKonowalczyk MarcinKonowalczyk marked this pull request as ready for review April 27, 2024 12:34
@gaborbernat gaborbernat merged commit 3db9822 into tox-dev:main Apr 27, 2024
25 checks passed
@gaborbernat
Copy link
Member

Thank you for your contribution.

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

Successfully merging this pull request may close these issues.

None yet

2 participants