Downgrade pytest back to 7.4.3 #763
Merged
Conversation
This file contains 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
Specifically to see if it resolves the qualification run timeout issues. Also commented out a pytest 8.0.x feature usage in xbgpu's unit test to ensure Jenkins doesn't complain about this as well. Contributes to: NGC-1318.
To remove dependency on pytest 8.x.x as we need to constrain the qualification test requirement to version<8.x.x. Contributes to: NGC-1318.
Citing pytest issue #12328 raised by @bmerry regarding fixture reuse. Contributes to: NGC-1318.
From the temporary 23 hours, but still greater than the original 5 hours. Contributes to: NGC-1318.
bmerry
requested changes
May 16, 2024
test/xbgpu/test_engine.py
Outdated
Comment on lines
1048
to
1051
| assert list(filter(accum_warning_filter, caplog.record_tuples))[: len(corrprod_outputs)] == [ | ||
| ( | ||
| "katgpucbf.xbgpu.engine", | ||
| WARNING, |
There was a problem hiding this comment.
You're repeating the value you're checking for in both the filter function and in this test.
How about replacing this with something like:
assert caplog.record_tuples.count(
(
"katgpucbf.xbgpu.engine",
WARNING,
"All Antennas had a break in data during this accumulation",
)
) >= len(corrprod_outputs)There was a problem hiding this comment.
Yeah fair point. Realised that when I explicitly compared tuples in the filter. Updating now.
bmerry
approved these changes
May 16, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
As pytest-dev/pytest#12328 mentions, the recent upgrade to pytest 8.x.x
resulted in the qualification tests taking forever and a day to complete (well, ~12 hours).
This PR rolls the version back to the last known working iteration and updates the pytest
8 feature usage.
There is also a note in
qualification/requirements.into support the reasoning to keep its pytest versionbelow 8. At least for now, until there is a new release of pytest with the fix for the issue raised by @bmerry.
Lastly, there is a corresponding tweak to the Jenkins Qualification pipeline to start the qualification run
at 11pm SAST. With the 8 hour timeout, that should take it through to a reasonable hour before the team
checks in.
Checklist (if not applicable, edit to add
(N/A)and mark as done):setup.cfgand.pre-commit-config.yamlsphinx-apidoc -efo doc/ src/to update files indoc/fake_servers.pyin katsdpcontroller to matchCloses NGC-1318.