Skip to content

SIPN/A Refactor system-level tests into new directory - #422

Merged
Alomir merged 3 commits into
masterfrom
SIPna-move-system-tests-to-own-dir
Oct 27, 2026
Merged

Alomir merged 3 commits into
masterfrom
SIPna-move-system-tests-to-own-dir

Conversation

@Alomir

@Alomir Alomir commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • What: Move 'system level' tests into a new test directory
  • Motivation: test_modeling is getting out of hand, and some of the tests are at a different level.

Here, my rule of thumb for a "system test" is: does this test call updateState() as part of it's flow?

How was this change tested?

All tests run and pass

Related issues

  • Fixes #N/A

Checklist

  • Related issues are listed above. PRs without an approved, related issue may not get reviewed.
  • PR title has the issue number in it ("[#] <concise description of proposed change>")
  • Tests added/updated for new features (if applicable)
  • Documentation updated (if applicable)
  • docs/CHANGELOG.md updated with noteworthy changes
  • Code formatted with clang-format (run git clang-format if needed)

Note: See CONTRIBUTING.md for additional guidance. This repository uses automated formatting checks; if the pre-commit hook blocks your commit, run git clang-format to format staged changes.

@Alomir
Alomir marked this pull request as ready for review October 27, 2026 19:38
Copilot AI balanced review requested due to automatic review settings October 27, 2026 19:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The event fixture is not available in the test’s working directory, causing the test to pass without exercising the event-file-based leaf-on or leaf-off balance checks.

No open issues 🎉

What changed in this PR

This PR separates timestep-level regression tests from focused modeling tests by moving tests that call updateState() into test_system.

Changes:

  • Relocates balance, complete-harvest, and limitation tests with their climate and parameter fixtures.
  • Adds a system-test Makefile and removes the relocated tests from the modeling suite.
  • Updates IDE source paths and allows test Makefiles through .gitignore.
File Description
tests/​sipnet/​test_system/​testLimitations.c Relocates resource-limitation regression tests.
tests/​sipnet/​test_system/​testCompleteHarvest.c Relocates harvest and related event regression tests.
tests/​sipnet/​test_system/​testBalance.c Relocates carbon and nitrogen balance tests.
tests/​sipnet/​test_system/​termination.param Supplies parameters for harvest and limitation tests.
tests/​sipnet/​test_system/​termination.clim Supplies the three-step climate fixture.
tests/​sipnet/​test_system/​Makefile Builds and runs the new system-test suite.
tests/​sipnet/​test_system/​balance.param Supplies balance-test parameters.
tests/​sipnet/​test_system/​balance.clim Supplies the five-day balance-test climate fixture.
tests/​sipnet/​test_modeling/​Makefile Removes relocated tests from the modeling suite.
CMakeLists.txt Updates IDE references to relocated test sources.
.gitignore Allows Makefiles in test-suite directories.

Review effort: Balanced

@Alomir
Alomir merged commit 6a54745 into master Oct 27, 2026
12 checks passed
@Alomir
Alomir deleted the SIPna-move-system-tests-to-own-dir branch October 27, 2026 20:57
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.

2 participants