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

Add regression tests #29

Merged
merged 15 commits into from
Nov 28, 2019
Merged

Add regression tests #29

merged 15 commits into from
Nov 28, 2019

Conversation

ofuhrer
Copy link
Contributor

@ofuhrer ofuhrer commented Nov 25, 2019

This PR adds regression tests to the test suite repository. The regression tests use a surrogate model model.py which emulates the behavior of the COSMO model and is used for rapid regression testing. Some fixes had to be applied to the testsuite Python scripts in order to make all tests pass. Would be great if we could add this to the testing plan (which I don't think I have access to).

Note, to run these tests on Piz Kesch, you can simply execute make test in the top-level testuite directory (see image below for results).

image

@ofuhrer ofuhrer changed the title Feature/regression tests Add regression tests Nov 25, 2019
@ofuhrer
Copy link
Contributor Author

ofuhrer commented Nov 25, 2019

image

@ofuhrer
Copy link
Contributor Author

ofuhrer commented Nov 25, 2019

This PR is done and ready to review. Could @lxavier or @kosterried pick this up. Should not be a big thing, since it basically adds regression testing. The edits in the testsuite Python code are minimal.

@ofuhrer ofuhrer mentioned this pull request Nov 25, 2019
@lxavier
Copy link
Contributor

lxavier commented Nov 27, 2019

@ofuhrer seems to work, but I get an error on kesch when doing make test, see below:

test_regression.py::test_full_testlist_from_xml PASSED

=================================================== 39 passed in 47.93s ===================================================
Exception ignored in: <bound method TemporaryDataDirectory.__del__ of <test_regression.TemporaryDataDirectory object at 0x2aaab4bd2240>>
Traceback (most recent call last):
  File "/scratch-shared/meteoswiss/scratch/lapixa/testsuite/tests/regression/test_regression.py", line 55, in __del__
AttributeError: 'NoneType' object has no attribute 'rmtree'

@ofuhrer
Copy link
Contributor Author

ofuhrer commented Nov 27, 2019

@lxavier Sorry, obviously my strategy for cleaning away the data directory did not work. Should work now. Removed that cleanup section. Will have to use pytest fixtures, but don't know how they work yet.

Copy link
Contributor

@lxavier lxavier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me, I was even able to make the test fail by changing the thresholds !
I am still trying to get our jenkins to work, I will merge once this is passed

@lxavier
Copy link
Contributor

lxavier commented Nov 28, 2019

@guydemorsier there is a PR from Oli but it fails with int2lm here, do you see what could be wrong :
http://jenkins-mch.cscs.ch/view/tools/job/testsuite/55/real_type=double,slave=kesch,target=gpu/console

@lxavier
Copy link
Contributor

lxavier commented Nov 28, 2019

the issue is in the was the testsuite is integrated in int2lm, I will fix it in int2lm.
I manually run with
int2lm test ok
cosmo test ok (except for output_tolerance), but this was in fact broken, I will open an issue

@@ -66,13 +68,17 @@ def check():
# check for output lists
try:
nout_list = get_param(rundir+nlfile, lnout)
assert(nout_list != '')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks to the assert, I saw this is in fact not working in some tests, see
#37

@lxavier
Copy link
Contributor

lxavier commented Nov 28, 2019

@kosterried and @guydemorsier I merge this, but I will need to fix some of the cosmo test see
#37
and change one configuraiton in int2lm

@lxavier lxavier merged commit f8a0f46 into C2SM-RCM:master Nov 28, 2019
@lxavier
Copy link
Contributor

lxavier commented Nov 28, 2019

@ofuhrer congratulation first external contribution :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants