✅ Add CI configs to run tests on Windows and MacOS#824
✅ Add CI configs to run tests on Windows and MacOS#824tiangolo merged 23 commits intofastapi:masterfrom
Conversation
|
There's one remaining issue on Windows, it happens in most of the [UPDATE: see "Windows encoding issue with |
This comment was marked as outdated.
This comment was marked as outdated.
|
Ok, the Windows & MacOS-specific issues seem to be addressed, but now the coverage report has broken 😭
I continue investigating... [UPDATE: fixed by using |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
tiangolo
left a comment
There was a problem hiding this comment.
Dang, you're good 🙇 😎
Impeccable job with all this! ✨
And all the data and references in the description... :chef_kiss: (we need that chef kiss emoji in GitHub).
Just a tiny nitpick, mainly to be annoying. 😅
|
📝 Docs preview for commit 324d1fb at: https://15f240fb.typertiangolo.pages.dev |
|
Amazing job, thank you! 🙇 🚀 |
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Extend test suite:
In total, this runs the CI two more times than before (7 instead of 5).
Fixes
To get the CI to go green for all systems, following edits/fixes were required:
Ensure we're always reading in UTF-8
tests/test_tutorial/test_parameter_types/test_file/test_tutorial004(_an)failed on Windows:To fix it, I set the
encodingforread_textto beutf8.Skip completion tests for Windows and MacOS
This isn't the cleanest of solutions, but it seems to me that these completion tests were very much written to work on Linux/bash only. In the future, it'd be nice to have MacOS & Windows compatible tests for this - I can make an issue for it to follow up later?
Use OS in coverage report name
By including the OS in the
coveragefile, we prevent overwriting / confusion:Windows encoding issue with
coverage runAdded an
envargument tosubprocess.runcalls that were failing for thetest_commands/test_helptest_scripttests. I'm still not clear why we were gettingUnicodeDecodeError's for these. I double checked all files in thetypercodebase and couldn't find any with a wrong decoding. Something must be happening when callingcoverage run. Either way, by taking inspiration from cpython#105312 and chaiNNer#2815 it seems to be fixed.MacOS issue with Python dir caching
The Python installation is being cached with
actions/cache@v3, which refers toenv.pythonLocation. However on macOS, this directory appears to be empty. The "Post Run" log would mentionand then the next run would appear to "successfully" restore from this cache, but the commands wouldn't actually be available:
Cf also a related discussion actions/setup-python#813. I decided to implement the same workaround as torchgeo/torchgeo#2024, basically disabling the caching for MacOS.
Windows issue with Python dir caching
One remaining issue is to investigate why the Windows caching isn't currently working:
This is not necessary to get this PR green, so I suggest to keep this as follow-up work.
Fix combining coverage files from different OS's
When combining the different
.coverage.XXXfiles from the different runs in the matrix,coverageneeds to remap the locations. It's easier to just use therelative_filesoption as suggested in the Coverage docs.