-
Notifications
You must be signed in to change notification settings - Fork 94
Switch to PyTest #642
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
base: master
Are you sure you want to change the base?
Switch to PyTest #642
Conversation
Mostly this modernizes/updates the way we checkout the submodules and the proper branches to canned actions.
Also moves test_storm for testing purposes. Adds a :TODO: comment on data.py to move from `os.path` to `pathlib`.
Yeah, we can update the regression data when necessary. |
I have done a review of all the tests and it appears that once a couple of bugs were fixed that the problems may now stem from the use of the modifications introduced in clawpack/riemann#183. Below are comparisons of the old test data and the new. ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() ![]() |
I also should add that I added an example/test to address the recent changes to the Riemann solver. The test is in vortex and implements the moving vortex test case. It currently is simply a regression/validation test, but we could add a convergence test to it. The more pressing matter though is where to put this example. I currently added a directory called |
Regarding where to put examples that aren't a tsunami, surge, etc., we could just put them in the top level of the examples directory, e.g. in |
@mandli: regarding your comment
I'm not sure I understand the plots. Several of them are labelled Gauge 1 and are very different from one another. Also one of them has a discontinuity in q[3,:] at t=0.5. What are these showing? |
Makes sense. Will change over and propose some examples to move into an upper directory. |
Sorry, should have descriped the plots better, but I was being lazy and automated all the comparisons. The test in question is also not clearly labeled (it's in the legend). Here are some clarifications:
I am not sure about the discontinuity. That is the dtopo test, which was failing even before the changes, but I am not as familiar with that code nor test. |
A clean attempt at switch the tests over to PyTest. Also moves the tests out of a dedicated directory and aligns each with an example, modifying only what needs to be modified.