MAINT: run ci builds on windows and macos #166
Conversation
|
At least some errors found in scipp/scippnexus#209 (workflow https://github.com/scipp/scippnexus/actions/runs/8831464559/job/24246765597) relate to the known int32 / int64 discrepancy between Unix and Windows. Those mostly show up because of how test data is defined. But we should make sure it is not caused by any library code. |
Yeah I guess the only to find it out is to start running CI on windows. |
template/.github/workflows/ci.yml
Outdated
| @@ -37,7 +37,7 @@ jobs: | |||
| needs: formatting | |||
| strategy: | |||
| matrix: | |||
| os: ['ubuntu-22.04'] | |||
| os: ['ubuntu-22.04', 'windows-latest'] | |||
There was a problem hiding this comment.
In the past, we had issues with windows builds taking much longer than linux or osx.
I don't remember if it was a compiling (scipp's C++ code) or a conda issue or something else, but I think we should check build times on various projects before going ahead with this.
It would not be great if everyone's builds just get a lot slower, in addition to the fact that we would be using two builders for the tests instead of 1 (and we only have 20 across the entire organisation).
I can see that building on PRs would be better than a weekly cron job (catch things as early as possible), so if the builds are not too long, we should do it. We should then maybe think again about paying for additional runners?
There was a problem hiding this comment.
Yes we can make this a weekly cron job if we see things are getting slow.
It shouldn't really take that long with the pure python projects.
There was a problem hiding this comment.
In my experience with Scitacean (Python only), the Windows jobs are significantly slower than Linux. But MacOs is even slower. So Windows is not such a problem when we don't have to compile C++.
Does the limit of 20 apply across OSs?
There was a problem hiding this comment.
Maybe we also have to get better with splitting our test suites into short and long tests? In particular in ESSsans there is a fair number of tests on larger files that add up to a significant runtime, and it will likely get worse.
There was a problem hiding this comment.
Does the limit of 20 apply across OSs?
Yeah, it's not per repo limit. It depends on the plan of the organisation.
Should I just make this a weekly cron job?
|
Should I just make this a weekly cron job? |
|
Putting this on the board so we don't forget about it. |
|
Now that this is weekly cron job I have added a windows and a macos test run. |
MridulS
changed the title
| - version: '${{needs.formatting.outputs.min_python}}' | ||
| tox-env: '${{needs.formatting.outputs.min_tox_env}}' |
There was a problem hiding this comment.
This won't work because there is no needs. We probably need an additional job or step to get the python version like we do in the ci.yml workflow:
- name: Get Python version for other CI jobs
id: vars
run: |
echo "min_python=$(cat .github/workflows/python-version-ci)" >> $GITHUB_OUTPUT
echo "min_tox_env=py$(cat .github/workflows/python-version-ci | sed 's/\.//g')" >> $GITHUB_OUTPUT| uses: ./.github/workflows/docs.yml | ||
| with: | ||
| publish: false | ||
| linkcheck: ${{ contains(matrix.variant.os, 'ubuntu') && github.ref == 'refs/heads/main' }} |
There was a problem hiding this comment.
I would say we don't need to run linkcheck on these builds? Would just add more noise (failed runs even if all tests have passed).
|
Is it possible to try and run this manually from the branch, just to check that it actually runs? |
Yes it has |
We shouldn't be skipping windows runs on CI.