Skip to content
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

Re-evaluating Skipped Unit Tests #376

Open
coreyostrove opened this issue Nov 30, 2023 · 3 comments
Open

Re-evaluating Skipped Unit Tests #376

coreyostrove opened this issue Nov 30, 2023 · 3 comments
Assignees
Labels
enhancement Request for a new feature or a change to an existing feature
Milestone

Comments

@coreyostrove
Copy link
Contributor

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.

@coreyostrove coreyostrove added the enhancement Request for a new feature or a change to an existing feature label Nov 30, 2023
@coreyostrove coreyostrove self-assigned this Nov 30, 2023
@rileyjmurray
Copy link
Collaborator

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.

@sserita sserita added this to the 0.9.14+ milestone Nov 30, 2023
@coreyostrove
Copy link
Contributor Author

coreyostrove commented Dec 5, 2023

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
Copy link
Contributor

jordanh6 commented Dec 5, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Request for a new feature or a change to an existing feature
Projects
None yet
Development

No branches or pull requests

4 participants