-
Notifications
You must be signed in to change notification settings - Fork 95
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
Test on macOS 13 and 14 #2249
Test on macOS 13 and 14 #2249
Conversation
64f9d86
to
fc502c8
Compare
563e953
to
5e1c5a0
Compare
I had to disable a few tests and three whole files to get the test suite to pass consistently, but the tests that do run still add up to ~64% coverage. To ensure that this is somewhat reliable I've again ran the test suite 50 times, half in normal order and the other half in reverse, which all passed without issue (see: https://github.com/Open-MSS/MSS/actions/runs/8340807936/job/22825515984). I think this is ready to get merged as a starting point for testing on different platforms. I would like this workflow to become one unified testing workflow for all platforms, which is why I've included running on ubuntu-latest as well. I will try to add Intel-based macOS and Windows to it in future PRs. |
# Even a module level skip is not enough, they need to be completely ignored. | ||
# TODO: fix those tests and drop the ignores | ||
run: micromamba run -n ci env QT_QPA_PLATFORM=offscreen pytest -v -n logical --durations=20 --cov=mslib | ||
--ignore=tests/_test_msui/test_sideview.py --ignore=tests/_test_msui/test_topview.py --ignore=tests/_test_msui/test_wms_control.py |
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.
is this 20 a total time? and what happens when the tests timeout of 10 is reached?
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.
While debugging a test I recently had seen that it sometimes gets into the http_auth dialog but that is not defined. It is not reproducable, it then enters the password there and fails ;)
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.
The 20 just means "print the duration of the 20 slowest tests". I copied that over from the other testing workflow. If the 10 minute step timeout is reached the workflow is aborted and marked as failed.
Yes, there are still some flaky tests left. The files that are disabled here seem to cause at least some of that, since I have seen the same symptoms that they exhibit on macOS also happen on Linux, just much less frequently which is why it isn't much of a problem in the other workflow.
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.
Note: I've increased the step timeout because the macOS 13 runners are a bit slower than the macOS 14 ones.
@@ -6,7 +6,7 @@ log_file = pytest.log | |||
log_file_level = DEBUG | |||
log_file_format = %(asctime)s %(levelname)s %(message)s | |||
log_file_date_format = %Y-%m-%d %H:%M:%S | |||
timeout = 30 | |||
timeout = 60 | |||
filterwarnings = | |||
# These namespaces are declared in a way not conformant with PEP420. Not much we can do about that here, we should keep an eye on when this is fixed in our dependencies though. |
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.
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 is unrelated to this PR and there is an issue for it already: #1919. As mentioned there it seems like the real culprit is PyFilesystem2.
I am considering merging #2288 into this PR, since that is just a single line change. I initially thought that Intel-based macOS would require more changes, but it turned out that it doesn't. |
5e1c5a0
to
a360358
Compare
a360358
to
3c8fdd9
Compare
The macOS 13 runners use Intel processors while the macOS 14 runners use Apple's ARM processors.
The test_tutorial_dir seems to be able to run into a 30 second timeout under normal operation on the macOS 14 runners, so this increases the timeout a bit to accommodate.
3c8fdd9
to
47d6eb6
Compare
I've added Intel-based macOS to this PR now. It is a bit slower, but seems to run reliably as well. This is ready for review again. |
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.
LGTM :)
Adds a GitHub Actions workflow to test on macOS 13 and 14 runners, i.e. Intel-based and Apple silicon-based runners.
Split of from #2207. Fixes #2284, fixes #2285.