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

Moved Unittest tester from FORCE #2382

Merged
merged 4 commits into from
Oct 29, 2024

Conversation

caleb-sitton-inl
Copy link
Collaborator

@caleb-sitton-inl caleb-sitton-inl commented Sep 30, 2024


Pull Request Description

What issue does this change request address? (Use "#" before the issue to link it, i.e., #42.)

#2381

What are the significant changes in functionality due to this change request?

This adds a tester to specifically deal with running tests through rook that use the unittest module. It was initially created in FORCE (see https://github.com/idaholab/FORCE/pull/35).


For Change Control Board: Change Request Review

The following review must be completed by an authorized member of the Change Control Board.

  • 1. Review all computer code.
  • 2. If any changes occur to the input syntax, there must be an accompanying change to the user manual and xsd schema. If the input syntax change deprecates existing input files, a conversion script needs to be added (see Conversion Scripts).
  • 3. Make sure the Python code and commenting standards are respected (camelBack, etc.) - See on the wiki for details.
  • 4. Automated Tests should pass, including run_tests, pylint, manual building and xsd tests. If there are changes to Simulation.py or JobHandler.py the qsub tests must pass.
  • 5. If significant functionality is added, there must be tests added to check this. Tests should cover all possible options. Multiple short tests are preferred over one large test. If new development on the internal JobHandler parallel system is performed, a cluster test must be added setting, in XML block, the node <internalParallel> to True.
  • 6. If the change modifies or adds a requirement or a requirement based test case, the Change Control Board's Chair or designee also needs to approve the change. The requirements and the requirements test shall be in sync.
  • 7. The merge request must reference an issue. If the issue is closed, the issue close checklist shall be done.
  • 8. If an analytic test is changed/added is the the analytic documentation updated/added?
  • 9. If any test used as a basis for documentation examples (currently found in raven/tests/framework/user_guide and raven/docs/workshop) have been changed, the associated documentation must be reviewed and assured the text matches the example.

Copy link
Collaborator

@dylanjm dylanjm left a comment

Choose a reason for hiding this comment

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

@caleb-sitton-inl can you modify a unit-test in raven to use this new tester so we have some testing on this new feature of the testing harness? Thanks!

@caleb-sitton-inl
Copy link
Collaborator Author

@caleb-sitton-inl can you modify a unit-test in raven to use this new tester so we have some testing on this new feature of the testing harness? Thanks!

@dylanjm For sure! No problem!

@moosebuild
Copy link

Job Test qsubs sawtooth on 8033dd2 : invalidated by @joshua-cogliati-inl

sawtooth back up

@dylanjm
Copy link
Collaborator

dylanjm commented Oct 17, 2024

@caleb-sitton-inl Looks like your test is failing the pylint portion with this error:

scripts/TestHarness/testers/RavenUnittest.py:1:0: C0114: Missing module docstring (missing-module-docstring)

Go ahead and fix that and recommit. Thanks!

@moosebuild
Copy link

Job Mingw Test on 2477df8 : invalidated by @GabrielSoto-INL

@GabrielSoto-INL
Copy link
Collaborator

Looks like the Mingw Test is failing due to a single test:
Failed tests\framework\InternalParallelTests\ExternalModelRay

Error message for the test (the relevant parts copied from the Civet test results):
UnitTesterError.txt

@joshua-cogliati-inl Do you know if this is a common error? I don't think anything that @caleb-sitton-inl added in this PR should be messing with the parallel tests. Seeing a lot of these types of errors in the stack trace:

2024-10-24 11:35:18,069	WARNING worker.py:2037 -- A worker died or was killed while executing a task by an unexpected system error. To troubleshoot the problem, check the logs for the dead worker. RayTask ID: 07c62520e0469d6fcd5afb0bf21a65dfd7e7dfd801000000 Worker ID: 3282d14636b7dd49f322ede02c5addec93f3773ad99e08f127805060 Node ID: a2fcdcce43123a4eca224c134aa1e9343746ea658095fa00c799f570 Worker IP address: 127.0.0.1 Worker port: 54432 Worker PID: 10448 Worker exit type: SYSTEM_ERROR Worker exit detail: Worker unexpectedly exits with a connection error code 10054. An existing connection was forcibly closed by the remote host. There are some potential root causes. (1) The process is killed by SIGKILL by OOM killer due to high memory usage. (2) ray stop --force is called. (3) The worker is crashed unexpectedly due to SIGSEGV or other unexpected errors.

@PaulTalbot-INL
Copy link
Collaborator

See #2392. Once we have approval to apply the exception to that test, we can merge.

Copy link
Collaborator

@dylanjm dylanjm left a comment

Choose a reason for hiding this comment

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

This PR is approved to merge. A test has been modified to utilize this new UnitTester rook module so it will be leveraged in all future tests. The Windows tests are currently failing due to an issue outside of this PR -- #2392 has approved merging PRs failing these specific tests until its resolved. There are some plugin failures that appear to be caused by something outside of this PR. The authors of the plugins have been notified.

@dylanjm dylanjm merged commit 0826f8a into idaholab:devel Oct 29, 2024
11 of 12 checks passed
@dylanjm dylanjm mentioned this pull request Oct 29, 2024
10 tasks
@caleb-sitton-inl caleb-sitton-inl deleted the unittest-tester branch October 30, 2024 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants