Conversation
- moving the fetch content tests to integration tests - making a multistage dockerfile to build fetch content tests - rework fortran packaging
Adding more information about the fetch content dockerfile
- Moves the fetch content tests into the existing docker action - Attempts to reuse building the base layer across the targets
K20shores
force-pushed
the
refactor_tuvx_tests
branch
from
7582e07 to
999c611
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #722 +/- ##
=======================================
Coverage 76.63% 76.63%
=======================================
Files 108 108
Lines 7849 7849
=======================================
Hits 6015 6015
Misses 1834 1834
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the test structure for MUSICA's Fortran interface, consolidating Docker builds and reorganizing tests into unit and integration categories to improve maintainability and reduce infrastructure complexity.
Key Changes:
- Consolidated multiple compiler-specific Dockerfiles into a single multi-stage
Dockerfile.fetch-content - Reorganized Fortran tests from
fetch_content_integrationinto properunitandintegrationtest directories - Unified module installation paths using
MUSICA_INSTALL_MOD_DIRvariable throughout the build system - Moved
get_musica_versionfunction fromutil.F90to a dedicatedmusicamodule
Reviewed changes
Copilot reviewed 27 out of 29 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| CMakeLists.txt | Introduced MUSICA_MOD_DIR and MUSICA_INSTALL_MOD_DIR variables for consistent Fortran module handling |
| cmake/dependencies.cmake | Added netcdf dependency handling and updated CARMA/TUV-x module directory configuration |
| cmake/setup_musica_target.cmake | Removed PUBLIC linking of object libraries, added netcdf linking, and fixed install interface paths |
| cmake/musicaConfig.cmake.in | Added netcdf dependency resolution for downstream consumers |
| cmake/musica.pc.in | Corrected include directory path in pkg-config file |
| src/packaging/CMakeLists.txt | Removed redundant install directives for object libraries and unified module installation |
| fortran/CMakeLists.txt | Simplified module handling by removing duplicate directory setup and custom copy commands |
| fortran/packaging/CMakeLists.txt | Updated module installation path to use MUSICA_INSTALL_MOD_DIR |
| fortran/musica.F90 | Renamed internal function to musica_get_version_c for better naming consistency |
| fortran/util.F90 | Removed get_musica_version function (moved to musica module) and cleaned up exports |
| fortran/test/CMakeLists.txt | Added integration test subdirectory |
| fortran/test/unit/CMakeLists.txt | Removed tests that were moved to integration directory |
| fortran/test/integration/CMakeLists.txt | New file organizing integration tests for MICM and TUV-x |
| fortran/test/integration/test_micm_api.F90 | Added error checking assertion |
| fortran/test/integration/test_micm_box_model.F90 | New comprehensive MICM box model test |
| fortran/test/integration/test_micm_multiple_grid_cells.F90 | New multi-grid cell test demonstrating parallel capabilities |
| fortran/test/integration/test_tuvx_api.F90 | Minor whitespace cleanup |
| fortran/test/tutorial/demo.F90 | Updated to use get_musica_version from musica module |
| fortran/test/test_simple.F90 | Deleted (superseded by better organized tests) |
| fortran/test/fetch_content_integration/test_get_micm_version.F90 | Deleted (functionality in tutorial/demo.F90) |
| fortran/test/fetch_content_integration/CMakeLists.txt | Refactored to use shared test sources from integration directory |
| docker/Dockerfile.fetch-content | New multi-stage Dockerfile supporting GCC, Intel, and NVHPC compilers |
| docker/Dockerfile.fortran-gcc | Deleted (consolidated into Dockerfile.fetch-content) |
| docker/Dockerfile.fortran-intel | Deleted (consolidated into Dockerfile.fetch-content) |
| docker/Dockerfile.fortran-nvhpc | Deleted (consolidated into Dockerfile.fetch-content) |
| docker/Dockerfile.fortran-gcc.integration | Modified to remove -j parallelism flag and memcheck option |
| docker/README.md | New documentation for Docker build process and multi-stage builds |
| .github/workflows/docker.yml | Consolidated fetch_content tests into docker.yml and added multi-compiler testing |
| .github/workflows/fetch_content_integration.yml | Deleted (merged into docker.yml) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
boulderdaze
approved these changes
Jan 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.