Refactor/testing #98

MishaZakharchanka · 2021-10-14T16:44:11Z

This pull request deals with a few changes/additions to Spheral's testing:
-Moving all tests into a separate testing directory
-Adding C++ tests to the integration.ats
-installing all the tests to be in the same location as our python install

…e tests directory.

…o install dir

… to Test dir.

…ral tests.

…urce directory.

mdavis36 · 2021-10-15T17:57:41Z

cmake/spheral/SpheralAddLibs.cmake

-                        INSTALL_RPATH "${CMAKE_INSTALL_PREFIX}/lib;${boost_DIR}/lib;${python_DIR}/lib"
+                        INSTALL_RPATH "${CMAKE_INSTALL_PREFIX}/lib;${boost_DIR}/lib;${python_DIR}/lib;${conduit_DIR}/lib"


Revert this change we shouldn't need to include the conduit lib directory in our rpaths any more.

…new ones created.

mdavis36 · 2021-10-21T15:00:32Z

tests/Functional/Hydro/Noh/Converge.txt

+100 0.0510675624557
+100 0.109837589561
+100 0.0510675624557
+100 0.109837589561
+100 0.109837589561
+100 0.109837589561


@jmikeowen Is this a file that we usually have in the spheral source or is it generated by the tests, I know it sometimes shows up in my git status reports. If it's generated then we should remove it.

That is generated by the some of the Noh tests when they run. It should be treated the same way we treat any other output from such things I suppose. It's not part of the repot.

mdavis36

There are a couple of files where it looks like there is still a bad merge conflict, all of the test file changes should show up essentially as name changes. It also appears that there are some generated test result/output files that are getting added into this new directory structure from the old one

mdavis36 · 2021-10-21T15:05:14Z

tests/Functional/Hydro/Sod/Sod-planar-1d-WaterGas.py

-# GSPH
-#
-#ATS:gsph1 = test(         SELF, "--gsph True --nx1 500 --nx2 30 --cfl 0.45 --graphics None --clearDirectories True  --restartStep 20 --steps 40", label="Planar Water-Gas Sod problem with GSPH -- 1-D (serial)")
-#ATS:gsph2 = testif(gsph1, SELF, "--gsph True --nx1 500 --nx2 30 --cfl 0.45 --graphics None --clearDirectories False --restartStep 20 --steps 20 --restoreCycle 20 --checkRestart True", label="Planar Water-Gas Sod problem with GSPH -- 1-D (serial) RESTART CHECK")
-#
-


@MishaZakharchanka It looks like there are still some merge conflicts to resolve in this file and a few others.

MishaZakharchanka · 2021-10-21T19:34:43Z

I believe I have fixed the merge problems and removed the generated files from the branch. All tests pass for me, let me know if there are any other changes/problems.

jmikeowen

This looks good to me. One question for the future -- do we want to support different methods of install? For instance, should we support installing just the necessary bits to run the code when installing public releases? Currently installing the tests/ directory isn't too onerous, but if we add more reference data to test for instance that size could grow.

mdavis36 · 2021-10-21T21:42:04Z

This looks good to me. One question for the future -- do we want to support different methods of install? For instance, should we support installing just the necessary bits to run the code when installing public releases? Currently installing the tests/ directory isn't too onerous, but if we add more reference data to test for instance that size could grow.

I think it's reasonable to have a flag for installing tests or not. ENABLE_TESTS is the conventional flag given for building tests so I don't see why we couldn't use it for the install logic with the rest of the python tests and reference data.

@MishaZakharchanka can you add a check around the install and "add_subdirectory" logic in CMakeLists.txt to see if ENABLE_TESTS is true.

mdavis36

Looking at a clean clone of the directory structure there still seems to be a number of src/***/test/ directories floating around please move those over to test/unit/.

Can we change the Unit and Functional directories to lowercase. The only place we don't do that is our src/* directories as they represent their respective module names.

@jmikeowen in our functional tests we have EvardSphericalCollapse, KeplerianPressureDisk and SelfSimilarCollapse floating around on their own. Is there a directory they might be better placed or should we just leave them as is?

mdavis36 · 2021-10-28T15:45:46Z

CMakeLists.txt

+if (ENABLE_TESTS)
+  add_subdirectory(${SPHERAL_ROOT_DIR}/tests/Unit/CXXTests)
+
+  install(DIRECTORY ${SPHERAL_ROOT_DIR}/tests/
+        DESTINATION ${CMAKE_INSTALL_PREFIX}/tests
+        PATTERN "runCXXTests.ats" EXCLUDE
+       )
+endif()


Can we move this logic into SetupSpheral.cmake as this logic will be needed in LLNLSpheral as well.

Those are all variations on tests that include gravity+hydro. For now keep 'em as is, I suppose we could add a hydrodyanmics and gravity test directory as well at some point.

Mikhail Zakharchanka and others added 18 commits September 3, 2021 16:09

Moving testing directories used in integration.ats out of src into th…

2640816

…e tests directory.

copying testing directory into cmake install prefix

0b21ef0

Merge branch 'develop' into refactor/testing

5fdda67

adding script to run c++ tests and installing the tests and scripts t…

70517fc

…o install dir

Merge branch 'develop' into refactor/testing

6215bb1

testing addition of SidreStorage_test added manually

6df5896

fixing merge conflict

04a81e7

working on getting SidreStorageTest working

7c0ac28

test added to integration.ats

458d326

Adding file for list of python test files

a2e5eea

merging in changes from develop

2bfa59e

making list of tests shorter

3b1d8df

Merge branch 'develop' into refactor/testing

16d282e

C++ tests added to integration.ats, need to move CXXTests from source…

b6e4f84

… to Test dir.

Merge branch 'develop' into refactor/testing

b0c83ac

Moved CXXTests out of src and into tests dir

617539e

Removed _test from C++ tests to match naming styling of existing Sphe…

c79c5c2

…ral tests.

Added empty file for intergration.ats to not fail if used from the so…

7abedaf

…urce directory.

mdavis36 reviewed Oct 15, 2021

View reviewed changes

Mikhail Zakharchanka and others added 5 commits October 15, 2021 11:23

Don't need conduit lib in rpath

92e88bf

Organized the tests dir into functional and unit directories.

8f842e4

Merge branch 'develop' into refactor/testing

eac9f84

Tests where moved and renamed, but git thinks files were deleted and …

71a193f

…new ones created.

Removed fake tests, uncommented integration.ats.

fe3a7a8

MishaZakharchanka marked this pull request as ready for review October 20, 2021 22:12

MishaZakharchanka requested review from jmikeowen and mdavis36 October 20, 2021 22:12

Mikhail Zakharchanka added 2 commits October 20, 2021 15:18

fixing merge conflicts

897e57e

fixing merge conflicts

015fa81

mdavis36 reviewed Oct 21, 2021

View reviewed changes

mdavis36 requested changes Oct 21, 2021

View reviewed changes

Mikhail Zakharchanka added 4 commits October 21, 2021 12:02

working on merge conflict problems.

3b7b874

Removing generated files from commit.

1bb7227

Removing more generated files.

a80c7b9

Adding a few comments to explain some of the files.

23a37b3

MishaZakharchanka requested a review from mdavis36 October 21, 2021 19:35

jmikeowen approved these changes Oct 21, 2021

View reviewed changes

Mikhail Zakharchanka added 2 commits October 21, 2021 15:19

Adding if ENABLE_TESTS so that tests dont have to be installed.

9e71686

Merge branch 'develop' into refactor/testing

3541527

mdavis36 requested changes Oct 28, 2021

View reviewed changes

Mikhail Zakharchanka and others added 3 commits October 28, 2021 10:28

moving over testing directories that remained in main.

8926714

removing generated file

05d1b20

moving test installation and changing some files to lowercase unit.

4975af4

mdavis36 approved these changes Nov 1, 2021

View reviewed changes

MishaZakharchanka merged commit 6de1215 into develop Nov 1, 2021

MishaZakharchanka deleted the refactor/testing branch November 4, 2021 23:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor/testing #98

Refactor/testing #98

MishaZakharchanka commented Oct 14, 2021

mdavis36 Oct 15, 2021

mdavis36 Oct 21, 2021

jmikeowen Oct 21, 2021

mdavis36 left a comment

mdavis36 Oct 21, 2021

MishaZakharchanka commented Oct 21, 2021

jmikeowen left a comment

mdavis36 commented Oct 21, 2021 •

edited

Loading

mdavis36 left a comment

mdavis36 Oct 28, 2021

jmikeowen Nov 1, 2021

		INSTALL_RPATH "${CMAKE_INSTALL_PREFIX}/lib;${boost_DIR}/lib;${python_DIR}/lib"
		INSTALL_RPATH "${CMAKE_INSTALL_PREFIX}/lib;${boost_DIR}/lib;${python_DIR}/lib;${conduit_DIR}/lib"

Refactor/testing #98

Refactor/testing #98

Conversation

MishaZakharchanka commented Oct 14, 2021

mdavis36 Oct 15, 2021

Choose a reason for hiding this comment

mdavis36 Oct 21, 2021

Choose a reason for hiding this comment

jmikeowen Oct 21, 2021

Choose a reason for hiding this comment

mdavis36 left a comment

Choose a reason for hiding this comment

mdavis36 Oct 21, 2021

Choose a reason for hiding this comment

MishaZakharchanka commented Oct 21, 2021

jmikeowen left a comment

Choose a reason for hiding this comment

mdavis36 commented Oct 21, 2021 • edited Loading

mdavis36 left a comment

Choose a reason for hiding this comment

mdavis36 Oct 28, 2021

Choose a reason for hiding this comment

jmikeowen Nov 1, 2021

Choose a reason for hiding this comment

mdavis36 commented Oct 21, 2021 •

edited

Loading