Conversation
|
Not sure why the pyodide test is failing. |
joerick
left a comment
There was a problem hiding this comment.
These comments are more documentation than tests, but it's nice to have them checked to ensure that they're not going out of date.
There's a similar map in oci_container_test that could be replaced with OCIPlatform.native().
Could you also add doctest checking to the run_tests.py and the nox tests task? That way we can avoid the doctests falling out-of-date. Happy to use xdoctest for that if you think it's suitable!
|
I'm fine with xdoctest FWIW, testing dependencies are low cost. |
|
I haven't added xdoctest integration to a project where nox was the driver. I think what I did is idomatic, but let me know if there is a way that makes more sense for cibuildwheel. I've added a similar invocation for doctests to the bin/run_tests.py script. There are two things that need to happen to get doctests running with xdoctest:
Looking at the noxfile it looked like the unit test and bigger tests were separated, so I added a third call for doctests only, which points at I also added Looks like circleci has a "-k" filter that disables all doctests, and if pytest collects no tests, then it returns an exitcode of 5. So, I've added a special case to allow for that to happen. The alternative is that we could add "or cibuildwheel" to Other changes:
|
Co-authored-by: Joe Rickerby <[email protected]>
Co-authored-by: Joe Rickerby <[email protected]>
Co-authored-by: Joe Rickerby <[email protected]>
e21ad3f to
54f73f4
Compare
I noticed that the
OCIContainerEngineConfigdoctest I wrote was no longer working. It seems like the class now requires oci_platform as a required arugment.To fix this, I added a classmethod to the enum to auto-detect what the current platform is. Then if we are on a supported platform it runs the test. If not, it gracefully skips the test with pytest.
This effect could also be achieved by using an xdoctest directive:
# xdoctest: +REQUIRES(LINUX)(assuming xdoctest was the doctest runner). However, I don't think these doctests are running in CI at all, otherwise this likely would have been caught.I also noticed two other doctests were failing. One in OptionsReader because there is no setup for the config_file. I didn't see an obvious way to make concise demo data for it, so I just added a skip directive.
The other doctest in util.helpers.format_safe seems to be broken. The doctest was testing for a backslash, but the logic seems to be wanting a hash as the escape character. This was easy to fix. With that
xdoctest cibuildwheelhas 3 doctests pass and 1 skip. This should also be easy to add to the CI by includingxdoctestas a dependency and using--xdoctestin the pytest invocation. It is possible to get these working with stdlib doctest, but xdoctest is a quite a bit more powerful and allows for more concise doctests (biased opinion).