-
Notifications
You must be signed in to change notification settings - Fork 248
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
feat: add --only #1098
feat: add --only #1098
Conversation
Hmm. I'm thinking more about this and I wonder if perhaps |
Ah, in fact the issue might be deeper than that, for development machine use. This comment raises another potential confusion, which is that That makes me think that maybe we should have a flag that's more designed around the 'testing locally' use-case... and that it should override all the build selector arguments
Names could be |
|
|
cd0c58e
to
1327351
Compare
6949cbd
to
111a37a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall structure looks good!
@pytest.mark.parametrize("only", ("cp311-manylxinux_x86_64", "some_linux_thing")) | ||
def test_only_failed(intercepted_build_args, monkeypatch, only): | ||
monkeypatch.setattr(sys, "argv", sys.argv + ["--only", only]) | ||
monkeypatch.delenv("CIBW_PLATFORM") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious why these delenvs are needed? Also on line 244, and 254.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because intercepted_build_args
uses platform
which injects it. The point of this is to see if adding something like --platform
or --archs
fails or some broken --only
is used, but instead it will fail because CIBW_PLATFORM
is set & --only
is present, and passes the test for the wrong reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've removed the coupling of intercepted_build_args from platform - it now monkeypatches all three. I'll push that to this PR so you can have a look.
See https://docs.python.org/3/library/re.html#re.escape Signed-off-by: Henry Schreiner <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>
111a37a
to
e472959
Compare
based on a profile run, this speeds up the unit tests by 3x
I've just pushed a few commits. After removing the Let me know if that works for you! edit: |
The CI failures are the
Having seen more of this recently I investigated a little #1254 |
That's fine with me - IMO it should be really rare to have |
Added |
Signed-off-by: Henry Schreiner <[email protected]>
3a50d0a
to
a9a2768
Compare
Closes #1087 by adding a
--only
arg. Closes #1262. By providing an alternative that only uses the wheel identifier.