Qiskit / rustworkx

A high performance Python graph library implemented in Rust.
https://www.rustworkx.org
Apache License 2.0
1.06k stars 146 forks source link

Switch from `unittest` to `stestr` runner after Windows 3.12 bug is fixed #1041

Open IvanIsCoding opened 9 months ago

IvanIsCoding commented 9 months ago

Information

What is the current behavior?

The test passes when running with python -m unittest discover and fail with stestr run. This indicates a possible issue with the test runner

Steps to reproduce the problem

Compare the result of this Github Action: https://github.com/Qiskit/rustworkx/actions/runs/7243374909 Against this Github Action: https://github.com/Qiskit/rustworkx/actions/runs/7243502875

It looks like skipped tests are failing stestr in Windows with Python 3.12

woodsp-ibm commented 9 months ago

I had noticed a similar failure in recent qiskit algorithms nightly builds - e.g here https://github.com/qiskit-community/qiskit-algorithms/actions/runs/7242612061/job/19728274840 One difference I had noticed is that when it ran ok it was Python 3.12.0 and when it started failing it was 3.12.1 in the log messages. I had looked at stestr, since that is used there too, but it has not been updated in some while. I had not investigated further

IvanIsCoding commented 9 months ago

I had noticed a similar failure in recent qiskit algorithms nightly builds - e.g here https://github.com/qiskit-community/qiskit-algorithms/actions/runs/7242612061/job/19728274840 One difference I had noticed is that when it ran ok it was Python 3.12.0 and when it started failing it was 3.12.1 in the log messages. I had looked at stestr, since that is used there too, but it has not been updated in some while. I had not investigated further

It could be an issue with Python 3.12.1 and one of the stestr or its dependencies. Minor versions shouldn’t break like this but out of all libraries that use undocumented behaviour from Python, I think this is one of the most likely cases

mtreinish commented 8 months ago

FWIW, this was a breaking api change made in Python 3.12.1's unittest module (which IMO was inappropriate for backporting in a stable point release, but there's no arguing with it now). I pushed up: https://github.com/testing-cabal/testtools/pull/371 which I think should fix it, but the failure mode I was fixing in that PR was slightly different from what's in the failure logs. But I expect it's the same root cause.

IvanIsCoding commented 8 months ago

FWIW, this was a breaking api change made in Python 3.12.1's unittest module (which IMO was inappropriate for backporting in a stable point release, but there's no arguing with it now). I pushed up: testing-cabal/testtools#371 which I think should fix it, but the failure mode I was fixing in that PR was slightly different from what's in the failure logs. But I expect it's the same root cause.

We can try it again after the fix is released. Currently the default runner is working well enough for GitHub CI so there is no rush