search

sandialabs / pyGSTi

A python implementation of Gate Set Tomography
http://www.pygsti.info
Apache License 2.0
134 stars 56 forks source link

Re-evaluating Skipped Unit Tests #376

Open coreyostrove opened 10 months ago

coreyostrove commented 10 months ago

I think all of the unit testing updates and conversations I've been a part of primed me to finally take proper notice of this, but we have a lot of unit tests being skipped presently. I clocked this locally for the main unit testing module as 73 tests and on the runners as 77 (I think 3 tests of the difference are accounted for by the mongoDB tests which aren't run on github, not sure about the fourth). I also clocked 12 skipped tests in the test extras module. These are spread throughout the testing module, but there are some clusters.

We should re-evaluate these to see if any of these should be resuscitated. If not we should think about whether we want to remove them.

Just as an example, at present the entire contents of the 'test\unit\extras\rb' directory (20-30 tests within) are being skipped. The comment tied to all of those is "RB analysis is known to be broken. Skip tests until it gets fixed." Is this comment still true? Perhaps it is but moot due to refactors in the RB codebase or these tests might be covered by existing RB tests in other modules. Or perhaps it isn't true and there are some tests in there providing coverage for things we're currently missing. (@jordanh6, you're one of the owners of this part of the codebase so may be in a position to tackle this if you're willing).

@rileyjmurray, you've been working a lot on the testing infrastructure recently so looping you in in case you're interested/willing to help with this.

rileyjmurray commented 10 months ago

I'm happy to help with this in whatever ways I can.

One thing I can say: there are a decent number of unit tests that call a function and perform no check for correctness of the output. They basically just verify that calling the function doesn't produce an error. These are better than nothing, but they really ought to be completed into proper correctness tests. LMK if it'd be helpful for me to track these down.

coreyostrove commented 10 months ago

In addition to the intentionally skipped tests, I'm bookmarking the fact that it looks like the contents of demoCalcMethods2Q in test_packages\drivers isn't being picked up by pytest (and so inadvertently skipped, probably because of the naming scheme). This set of tests looks like it hasn't been touched since before the last major refactor (i.e. pre-0.9.10, I think?) so if we decide we want to revive these they'll need to be brought up to day.

Edit: Also bookmarking test_mpi. This is currently not being tested with pytest by design (it needs to be run using MPI), but this means it probably is only very rarely being run, if ever.

jordanh6 commented 10 months ago

I can help tackle the skipped RB tests.

I took a quick glance and there appears to be a bit of overlap with tests we added recently, but also lots of tests for parts of the RB code that aren't covered by other unit tests. Most of the skipped tests look extremely old and at least need updating based on RB code refactoring.