-
Notifications
You must be signed in to change notification settings - Fork 120
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
[develop] Move all unittest tests to a common area. #728
[develop] Move all unittest tests to a common area. #728
Conversation
Turn off the retrieve_data tests in the unit_tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on Hera, and it works fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@christinaholtNOAA I have created a sandbox pipeline in Jenkins for testing PRs with modified Jenkinsfiles. The test failed in the initialization step due to a missing '
at the end of Functional Tests
on line 127. I also noted the lack of bash --login
and "
on line 130, which will likely cause problems as well. Once these changes have been made, I'll attempt to run the Jenkins tests again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@christinaholtNOAA Jenkins doesn't like the location of parallel (it expected stage, not parallel). I've provided an example of what the documentation would suggest a parallel build would look like, but I have no idea how to include post. Would it be okay to remove parallel {
from line 125 and just have a three-part pipeline?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@christinaholtNOAA Thank you very much for applying the changes to the Jenkinsfile! Jenkins is now able to run the updated pipeline with the Functional Tests section. However, the Functional Tests section is failing. The failure appears to be due to lines 8 and 9 in srw_unittest.sh
. I have added a suggestion that should correct the issue.
@christinaholtNOAA PR #736 was merged this morning. This updated the test_retrieve_data.py unittest script. This caused a conflict in your PR. Please merge your PR to the latest develop. |
@MichaelLueken I resolved the conflicts from last week. Let me know if there are any remaining issues with the Jenkins file. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@christinaholtNOAA Running the new srw_unittest.sh
script through the Jenkins pipeline shows a few more issues. I have manually run the script on Hera and have provided feedback to address everything that I encountered.
However, the test is failing to run with the following error:
+ export PYTHONPATH=/scratch2/NAGAPE/epic/Michael.Lueken/srw_unittest/ush
+ PYTHONPATH=/scratch2/NAGAPE/epic/Michael.Lueken/srw_unittest/ush
+ python -m unittest /scratch2/NAGAPE/epic/Michael.Lueken/srw_unittest/tests/test_python/test_retrieve_data.py
E
======================================================================
ERROR: /scratch2/NAGAPE/epic/Michael (unittest.loader._FailedTest)
----------------------------------------------------------------------
ImportError: Failed to import test module: /scratch2/NAGAPE/epic/Michael
Traceback (most recent call last):
File "/scratch1/NCEPDEV/nems/role.epic/miniconda3/4.12.0/envs/regional_workflow/lib/python3.9/unittest/loader.py", line 154, in loadTestsFromName
module = __import__(module_name)
ModuleNotFoundError: No module named '/scratch2/NAGAPE/epic/Michael'
----------------------------------------------------------------------
Ran 1 test in 0.000s
Have you seen this behavior before? I'll try to continue digging into this, but I can't tell if there is an issue with the test, the machine, or in the conda environment itself.
Co-authored-by: Michael Lueken <[email protected]>
Co-authored-by: Michael Lueken <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@christinaholtNOAA With the latest changes that you pushed to your branch, I'm now able to manually run the new srw_unittest.sh
script. I will now resubmit this PR in the Jenkins pipeline and see how it goes. Thanks for working with me on this work!
@christinaholtNOAA The new Functional Tests section is running on Hera. This section, however, failed on Jet with the following error:
It looks like using:
should allow the script to run on both Hera and Jet. Due to the nature of the testing, a failure will stop the entire pipeline, so this will be good to also ensure that everything that we are testing with test_retrieve_data.py is behaving as expected. This also means that the Jet and Hera tests will fail to run through due to HPSS being down until sometime tomorrow. |
We shouldn't need to I had a bit of trouble reproducing this failure as my own user, and finally figured out that it seems related to the I think it may work with a relative path and no
I will push the change to see if that works. |
I was able to manually run the Please note that I would like to run one last test once HPSS has been returned, to ensure that the functional tests pass with HPSS and AWS. Then, we can move forward with merging this work. |
@christinaholtNOAA It looks like the new Functional Tests section are working correctly for both Hera and Jet. The only issue is that the Functional Tests now take between one and two hours to run on Hera due to the inclusion from PR #736. The test is pulling several large NEMSIO files, significantly increasing run times for the two ufs-case-study pulls from AWS. Please note that this only appears to be an issue on Hera. The Jet test successfully passes in 17 minutes and the GitHub Actions test passes in 12 minutes. Issue #753 was opened detailing the work to correct this. As part of this PR, it might be a good idea to turn off the two ufs-case-study tests until the ICs/LBCs have been replaced by grib2 data. |
On Cheyenne GNU, the On Cheyenne Hera, the On Hera GNU, the On Hera Intel, the After rerunning the two failed tests on Hera GNU, so long as they pass, I will move forward with merging this work. |
Rerunning the |
DESCRIPTION OF CHANGES:
Moves all unittest tests to the tests directory, and lints them. They currently should pass at 10/10. This makes running them easier so that users may be encouraged and enabled to do that on their own platform.
Adds the HPSS functional tests to the Jenkinsfile, so that the full functionality is tested regularly. These tests run on the front end so shouldn't eat into allocation. The tests clean up after themselves, so shouldn't be a data issue in the long-run.
Adds linting for the tests directory to the GitHub Actions workflow to ensure they stay linted.
Type of change
TESTS CONDUCTED:
DEPENDENCIES:
None. Someone should check the Jenkinsfile though! I can't test that, really.
DOCUMENTATION:
Maybe?
ISSUE:
Fixes Issue #726.
CHECKLIST
LABELS (optional):
A Code Manager needs to add the following labels to this PR: