Refactor some tests to be only run under serial execution #376
Conversation
PhilipDeegan
force-pushed
the
serial-ize
branch
from
22120ee to
8ee2f27
Compare
PhilipDeegan
changed the title
PhilipDeegan
force-pushed
the
serial-ize
branch
3 times, most recently
from
3ff2bd4 to
bc4add0
Compare
PhilipDeegan
force-pushed
the
serial-ize
branch
2 times, most recently
from
92659d9 to
2a412d5
Compare
PhilipDeegan
force-pushed
the
serial-ize
branch
from
2a412d5 to
dd770cf
Compare
PhilipDeegan
force-pushed
the
serial-ize
branch
from
dd770cf to
6de96ae
Compare
There was a problem hiding this comment.
I don't know why this has waited so long.
I think it's to merge, do you @PhilipDeegan?
not exactly proriotiy, rebase to master then merge |
PhilipDeegan
force-pushed
the
serial-ize
branch
from
1702e64 to
5e75d20
Compare
WalkthroughWalkthroughThe changes primarily involve modifications to the CMakeLists.txt files across various directories in the project. The key change is the replacement of the Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Files selected for processing (8)
- tests/amr/data/field/coarsening/CMakeLists.txt (1 hunks)
- tests/amr/data/field/overlap/CMakeLists.txt (1 hunks)
- tests/amr/data/field/refine/CMakeLists.txt (1 hunks)
- tests/amr/data/field/refine/test_field_refinement_on_hierarchy.cpp (1 hunks)
- tests/amr/data/field/time_interpolate/CMakeLists.txt (1 hunks)
- tests/amr/data/field/variable/CMakeLists.txt (1 hunks)
- tests/amr/data/particles/copy/CMakeLists.txt (1 hunks)
- tests/amr/data/particles/copy_overlap/CMakeLists.txt (1 hunks)
Files skipped from review due to trivial changes (4)
- tests/amr/data/field/time_interpolate/CMakeLists.txt
- tests/amr/data/field/variable/CMakeLists.txt
- tests/amr/data/particles/copy/CMakeLists.txt
- tests/amr/data/particles/copy_overlap/CMakeLists.txt
Additional comments (Suppressed): 7
tests/amr/data/field/coarsening/CMakeLists.txt (1)
- 38-38: The function
add_phare_testhas been replaced withadd_no_mpi_phare_test. Ensure that this change does not affect the execution of the test suite and that the new function is defined and works as expected.tests/amr/data/field/overlap/CMakeLists.txt (1)
- 33-33: The function
add_phare_testhas been replaced withadd_no_mpi_phare_test. Ensure that this change is intentional and that the test does not require MPI support. If MPI support is required, consider creating a separate function for tests that require MPI.- add_phare_test(${PROJECT_NAME} ${CMAKE_CURRENT_BINARY_DIR}) + add_no_mpi_phare_test(${PROJECT_NAME} ${CMAKE_CURRENT_BINARY_DIR})tests/amr/data/field/refine/test_field_refinement_on_hierarchy.cpp (1)
- 135-152: The new
mainfunction initializes the Google Test framework and the SAMRAI framework, runs all tests, and then shuts down the frameworks. This is a standard setup for a test suite using these frameworks. However, ensure that the initialization and shutdown of the SAMRAI framework does not interfere with other tests or parts of the application that may also be using SAMRAI.tests/amr/data/field/refine/CMakeLists.txt (4)
18-34: The function
_setup_amr_field_refine_testis a good abstraction that encapsulates the common setup for the tests. It improves the modularity and maintainability of the code.37-40: The function
_add_general_amr_field_refine_testwraps the setup and addition of a test that requires MPI. This function improves the readability of the code and makes it easier to add new tests in the future.42-45: The function
_add_serial_amr_field_refine_testwraps the setup and addition of a test that does not require MPI. This function improves the readability of the code and makes it easier to add new tests in the future.48-49: Ensure that the tests
test_field_refinement_on_hierarchyandtest_field_refineare correctly categorized as requiring MPI and not requiring MPI, respectively. Misclassification could lead to test failures or incorrect test results.
PhilipDeegan
force-pushed
the
serial-ize
branch
from
5e75d20 to
9171647
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: .coderabbit.yaml
Files selected for processing (8)
- tests/amr/data/field/coarsening/CMakeLists.txt (1 hunks)
- tests/amr/data/field/overlap/CMakeLists.txt (1 hunks)
- tests/amr/data/field/refine/CMakeLists.txt (1 hunks)
- tests/amr/data/field/refine/test_field_refinement_on_hierarchy.cpp (1 hunks)
- tests/amr/data/field/time_interpolate/CMakeLists.txt (1 hunks)
- tests/amr/data/field/variable/CMakeLists.txt (1 hunks)
- tests/amr/data/particles/copy/CMakeLists.txt (1 hunks)
- tests/amr/data/particles/copy_overlap/CMakeLists.txt (1 hunks)
Files skipped from review due to trivial changes (1)
- tests/amr/data/field/refine/test_field_refinement_on_hierarchy.cpp
Additional comments: 10
tests/amr/data/particles/copy_overlap/CMakeLists.txt (1)
- 24-27: The change from
add_phare_testtoadd_no_mpi_phare_testindicates that the test no longer requires MPI. Ensure that this is the intended behavior and that the test still functions correctly without MPI.tests/amr/data/field/time_interpolate/CMakeLists.txt (1)
- 18-21: The change from
add_phare_testtoadd_no_mpi_phare_testsuggests that the test does not require MPI. Ensure that this is the case and that all necessary changes have been made to the test to accommodate this.tests/amr/data/field/coarsening/CMakeLists.txt (1)
- 34-38: The new custom command ensures that the Python script
test_coarsen_field.pyis run before the build of the target. This is a good practice for generating any necessary data or performing checks before the build process. However, ensure that the Python script does not have any side effects that could affect the build process or other tests. Also, verify that the Python script runs successfully in the build environment.tests/amr/data/field/variable/CMakeLists.txt (1)
- 23-26: The change from
add_phare_testtoadd_no_mpi_phare_testindicates that the test no longer requires MPI. Ensure that this is the intended behavior and that the test still functions correctly without MPI.- add_phare_test(${PROJECT_NAME} ${CMAKE_CURRENT_BINARY_DIR}) + add_no_mpi_phare_test(${PROJECT_NAME} ${CMAKE_CURRENT_BINARY_DIR})tests/amr/data/particles/copy/CMakeLists.txt (1)
- 24-27: The change from
add_phare_testtoadd_no_mpi_phare_testindicates that the test no longer requires MPI. Ensure that this is the intended behavior and that the test still functions correctly without MPI.- add_phare_test(${PROJECT_NAME} ${CMAKE_CURRENT_BINARY_DIR}) + add_no_mpi_phare_test(${PROJECT_NAME} ${CMAKE_CURRENT_BINARY_DIR})tests/amr/data/field/overlap/CMakeLists.txt (1)
- 30-34: The change from
add_phare_testtoadd_no_mpi_phare_testindicates that the test no longer requires MPI. Ensure that this is the intended behavior and that the test still functions correctly without MPI.- add_phare_test(${PROJECT_NAME} ${CMAKE_CURRENT_BINARY_DIR}) + add_no_mpi_phare_test(${PROJECT_NAME} ${CMAKE_CURRENT_BINARY_DIR})tests/amr/data/field/refine/CMakeLists.txt (4)
18-34: The function
_setup_amr_field_refine_testis well defined and modular. It sets up the test by adding an executable, setting the include directories, and linking the necessary libraries. This function is reused in_add_general_amr_field_refine_testand_add_serial_amr_field_refine_testwhich is a good practice.37-40: The function
_add_general_amr_field_refine_testsets up the test and adds it as a general test. It's clear and concise.42-45: The function
_add_serial_amr_field_refine_testsets up the test and adds it as a serial test. It's clear and concise.48-49: The new tests
test_field_refinement_on_hierarchyandtest_field_refineare added correctly using the new functions. Ensure that these tests are running and passing as expected.
* AMR Field Refine: extract some tests to be only run under serial execution * AMR Field Overlap: refactor tests to be only run under serial execution * AMR Field Time Interpolate: refactor tests to be only run under serial execution * AMR: rm tests from MPI suite
#358
not sure it's all the tests that could be "serialized" but it would probably make the mpi_tests quicker if it was merged as is
Summary by CodeRabbit
test_coarsen_field.pyto be run before the build, improving the build process.test_field_refinement_on_hierarchyandtest_field_refine, to improve the robustness of the software.test_field_refinement_on_hierarchy.cppto use Google Test, enhancing the reliability and readability of test results.