Make sure pytest actually fails CI on windows

[nwithan8]

For whatever reason, the error code from coverage run is not surfacing to the Windows runner, and therefore the CI is passing despite several unit tests failing.

Using pytest directly does surface the proper error code and halt CI if any tests fail, so that has been added as a step.

coverage run is still executed, after pytest, to generate the necessary information needed for coverage xml and coverage report. Because pytest runs first, the false-negative tests in coverage run will never be encountered.

This fails as expected:

Closes #777

[codecov]

Codecov Report

Merging #780 (79099bc) into master (4ae2ffe) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #780   +/-   ##
=======================================
  Coverage   21.51%   21.51%           
=======================================
  Files          63       63           
  Lines        8542     8542           
  Branches     1572     1572           
=======================================
  Hits         1838     1838           
- Misses       6680     6682    +2     
+ Partials       24       22    -2     

see 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[oddstr13]

I think it would be worth testing whether coverage run in a separate step properly fails the test
(are tests on windows only failing based on the exit code of the last command?)

[oddstr13]

Looks like my suspicion about only last command's exit code being used is likely correct, see nedbat/coveragepy#1003 and https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#exit-codes-and-error-action-preference

For now, just move coverage run to a separate step, naming it Run tests and collect coverage, and rename the existing one Generate coverage reports.

That should fix pytest not erroring (via coverage) on Windows.

---

We likely have similar issues all over CI, but that can be fixed properly in a separate PR, preferably by having multi-line test steps error out properly.

[oddstr13]

It is also possible, but not guaranteed that explicitly setting shell: bash on the run step(s) fixes the issue.

[nwithan8]

It is also possible, but not guaranteed that explicitly setting shell: bash on the run step(s) fixes the issue.

I don't believe that would work, trying to run a Bash shell on a Windows machine.

[nwithan8]

Looks like my suspicion about only last command's exit code being used is likely correct, see nedbat/coveragepy#1003 and https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#exit-codes-and-error-action-preference

For now, just move coverage run to a separate step, naming it Run tests and collect coverage, and rename the existing one Generate coverage reports.

That should fix pytest not erroring (via coverage) on Windows.

We likely have similar issues all over CI, but that can be fixed properly in a separate PR, preferably by having multi-line test steps error out properly.

Better Google-fu than me!

[oddstr13]

It is also possible, but not guaranteed that explicitly setting shell: bash on the run step(s) fixes the issue.

I don't believe that would work, trying to run a Bash shell on a Windows machine.

That shouldn't be a problem, as git is available (which depend on and include bash), and possibly also msys2/mingw (see actions/runner#497)

Your current change should be fine tho, but the xml and report steps should be in the same steps, as I don't expect either of those to ever fail

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information