-
-
Notifications
You must be signed in to change notification settings - Fork 381
Unbreak CI (3.14 + cython) #3264
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
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
8592150
skip static introspection on 3.14.0b1
jakkdl c36b62b
also skip sees_all_symbols, pin cython to <3.1.0
jakkdl 361fd07
name 3.14 with -dev to pass if failing. Also restrict cython version in
jakkdl d9e62f1
continue-on-error for windows3.14, add non-strict xfail for test_erro…
jakkdl 768ca85
switch back to skipif ... maybe unbreak CI?
jakkdl 20ff958
unbreak CI
jakkdl 6ec7436
summer is finally here ☀
jakkdl File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -117,6 +117,11 @@ def iter_modules( | |
| # won't be reflected in trio.socket, and this shouldn't cause downstream test | ||
| # runs to start failing. | ||
| @pytest.mark.redistributors_should_skip | ||
| @pytest.mark.skipif( | ||
A5rocks marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| sys.version_info[:4] == (3, 14, 0, "beta"), | ||
| # 12 pass, 16 fail | ||
| reason="several tools don't support 3.14", | ||
| ) | ||
| # Static analysis tools often have trouble with alpha releases, where Python's | ||
| # internals are in flux, grammar may not have settled down, etc. | ||
| @pytest.mark.skipif( | ||
|
|
@@ -243,6 +248,11 @@ def no_underscores(symbols: Iterable[str]) -> set[str]: | |
| @slow | ||
| # see comment on test_static_tool_sees_all_symbols | ||
| @pytest.mark.redistributors_should_skip | ||
| @pytest.mark.skipif( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here |
||
| sys.version_info[:4] == (3, 14, 0, "beta"), | ||
| # 2 passes, 12 fails | ||
| reason="several tools don't support 3.14.0", | ||
| ) | ||
| # Static analysis tools often have trouble with alpha releases, where Python's | ||
| # internals are in flux, grammar may not have settled down, etc. | ||
| @pytest.mark.skipif( | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
IMO it's cleaner to use 3.14-dev because it's implied that we should change it back once 3.14 stable is released.
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.
that didn't work https://github.com/python-trio/trio/actions/runs/14970947768/job/42051585912
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.
It still fails but then it's allowed to. (I think that run failed because the Ubuntu and MacOS failures, not the Windows one)
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.
uh, this is the error:
Uh oh!
There was an error while loading. Please reload this page.
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.
Oops, I wasn't able to see the logs. We should probably remove that logic near setup-python(I think it's only there because legacy) but for now this new thing is fine too.
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.
you're very right, I confused myself from some
git blameing.But regardless, let's just merge this as is to unblock other PRs and we can possibly clean this up later?
Uh oh!
There was an error while loading. Please reload this page.
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 personally don't like that
allow-prereleasesinput ofactions/setup-pythonand use3.14-devor more often~3.14.0-0to have it represent that it's not yet stable explicitly.One day, I'll show you my
reusable-tox.ymlthing which will simplify this by a lot...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 think
allow-prereleasesis fine, since basically it saves some unnecessary churn (we should be pretty sure Trio will not need changes before switching3.14-devto3.14) and allows marking some versions as allowed to fail just based on the version scheme.Is there a trap to be aware of?
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.
It's just that you're forced to keep updating the conditional on this line which is disconnected from the version string, even though they are tightly coupled through the concept of pre-releases. That's two places to keep in sync already with a visually unobvious link. This is what I consider mental overhead. The same churn just worse.
Having "allow-failures" in the same identifier allows being intentional about expecting a version to fail or not.
Uh oh!
There was an error while loading. Please reload this page.
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.
To be clear the only reason we had to update the conditional on this line is because some other legacy logic -- we don't actually have
allow-prereleasesbut instead some text interpolation to emulate the effects. The downside of the text interpolation is that we can't support3.14-dev, so we if switched toallow-prereleaseswe would have the same behavior as currently except that3.14-devwould work.And then since we have a
continue-on-errorwe can use the version identifier to signal whether failure is allowed (3.14-devwill allow failure,3.14won't). This way we can actually make use of the fact that Python tries not to break anything after they start releasing betas (by switching to3.14) while not having to switch the identifier whenever a stable version comes out.