-
Notifications
You must be signed in to change notification settings - Fork 150
Added tests on Windows #364
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| name: test-windows | ||
| run-name: Tests Windows | ||
|
|
||
| on: | ||
| workflow_dispatch: | ||
| pull_request: | ||
| push: | ||
| branches: [ main ] | ||
|
|
||
| defaults: | ||
| run: | ||
| shell: bash -e -l {0} | ||
|
|
||
| jobs: | ||
| test: | ||
| name: Run ctests | ||
| runs-on: windows-latest | ||
| strategy: | ||
| fail-fast: true | ||
| matrix: | ||
| json_version: [ develop, v3.12.0, v3.8.0 ] | ||
|
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. Should be more explicit if you are testing |
||
|
|
||
| steps: | ||
|
|
||
| - name: Setup MSVC | ||
| uses: ilammy/msvc-dev-cmd@v1 | ||
|
|
||
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Set conda environment | ||
| uses: mamba-org/setup-micromamba@main | ||
| with: | ||
| environment-name: myenv | ||
| environment-file: environment-dev.yml | ||
| init-shell: bash | ||
| cache-downloads: true | ||
| create-args: | | ||
| ninja | ||
|
Comment on lines
+31
to
+39
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. Please no conda/mamba. If you wish to check it make it a distinct test set. Instead for windows please use 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. The use of micromamba was to easily get the dependencies, I don't have the need to test it specifically and can use whatever you prefer. Out of curiosity, why are you opposed to using micromamba? 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.
Mostly that it is the wrong tool in CI. If the purpose is to test the conda ecosystem then it can make sense for that, but then it would be better to have the project packaged there. But for testing OS, using the primary package manager of that OS would be preferred, particularly important being the usage of the native compilers, in this case being msvc.
We are using FetchContent in order to have a package manager independent source of dependencies. Testing the package managers is also quite important since it can have specific patches applied, but in that case only the packaged packages must be used. |
||
|
|
||
| - name: Configure using cmake | ||
| run: cmake -Bbuild -DCMAKE_BUILD_TYPE:STING=Release -DCMAKE_INSTALL_PREFIX=$CONDA_PREFIX -G Ninja | ||
|
|
||
| - name: Build | ||
| working-directory: build | ||
| run: ninja | ||
|
|
||
| - name: Run tests | ||
| working-directory: build | ||
| run: ctest -R ^test$ --output-on-failure | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| name: json-schema-validator | ||
| channels: | ||
| - conda-forge | ||
| dependencies: | ||
| - cmake | ||
| - ninja | ||
| - nlohmann_json |
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.
Please re-use the
test.yamlinstead of creating dedicated workflow files. If you want to organize them in individual files, try a design like this.The main thing is that we should be gating different stages of the CI, e.g. always check a
pre-commitfirst before doing long tests.