From 8eeefb64a07bd767a61df32c103eb6f3be749d4c Mon Sep 17 00:00:00 2001 From: Jo Basevi Date: Fri, 28 Nov 2025 10:51:01 +1100 Subject: [PATCH 1/6] Add repro payu setup check - this should pick up changes to manifests --- .../config_tests/conftest.py | 4 +++ .../config_tests/test_bit_reproducibility.py | 14 +++++++++- src/model_config_tests/exp_test_helper.py | 28 +++++++++++++++++++ 3 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/model_config_tests/config_tests/conftest.py b/src/model_config_tests/config_tests/conftest.py index 74b98e1..5fe688d 100644 --- a/src/model_config_tests/config_tests/conftest.py +++ b/src/model_config_tests/config_tests/conftest.py @@ -131,6 +131,10 @@ def pytest_configure(config): "markers", "repro_determinism_restart: mark tests that check determinism restart", ) + config.addinivalue_line( + "markers", + "repro_payu_setup: mark tests that check payu setup reproducibility", + ) config.addinivalue_line("markers", "slow: mark tests that are slow to run") config.addinivalue_line( "markers", diff --git a/src/model_config_tests/config_tests/test_bit_reproducibility.py b/src/model_config_tests/config_tests/test_bit_reproducibility.py index c4852c4..1c69313 100644 --- a/src/model_config_tests/config_tests/test_bit_reproducibility.py +++ b/src/model_config_tests/config_tests/test_bit_reproducibility.py @@ -9,7 +9,7 @@ import pytest -from model_config_tests.exp_test_helper import Experiments, ExpTestHelper +from model_config_tests.exp_test_helper import Experiments, ExpTestHelper, setup_exp from model_config_tests.util import DAY_IN_SECONDS, HOUR_IN_SECONDS # Names of shared experiments @@ -328,3 +328,15 @@ def test_repro_determinism_restart( ) assert produced == expected + + +@pytest.mark.repro_payu_setup +def test_repro_payu_setup(control_path, output_path): + """ + Test payu setup with --reproduce which errors if payu manifests full hashes are changed + """ + experiment = setup_exp(control_path, output_path, exp_name="repro_payu_setup") + try: + experiment.setup_reproduce() + except Exception as error: + pytest.fail(f"{error}") diff --git a/src/model_config_tests/exp_test_helper.py b/src/model_config_tests/exp_test_helper.py index eaabbf5..5943d11 100644 --- a/src/model_config_tests/exp_test_helper.py +++ b/src/model_config_tests/exp_test_helper.py @@ -82,6 +82,34 @@ def has_run(self): """ return self.model.output_exists() + def setup_reproduce(self): + """ + Run payu setup command with --reproduce flag + """ + owd = Path.cwd() + # Change to experiment directory and run. + os.chdir(self.control_path) + + try: + setup_command = [ + "payu", + "setup", + "--lab", + str(self.lab_path), + "--reproduce", + ] + print(f"Running payu setup command: {setup_command}") + result = sp.run(setup_command, capture_output=True, text=True) + finally: + # Change back to original working directory + os.chdir(owd) + + if result.returncode != 0: + raise RuntimeError( + f"Failed to run payu setup with --reproduce. Error: {result.stderr}\n" + f"Full output: {result.stdout}" + ) + def setup_for_test_run(self): """ Various config.yaml settings need to be modified in order to run in the From c73ad17babe7120f0524347f373478b586490f7d Mon Sep 17 00:00:00 2001 From: Qianhui Chen Date: Fri, 13 Mar 2026 12:37:43 +1100 Subject: [PATCH 2/6] Use git diff to check the diff in manifest. --- src/model_config_tests/exp_test_helper.py | 28 +++++++++++++---------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/model_config_tests/exp_test_helper.py b/src/model_config_tests/exp_test_helper.py index 5943d11..50dbdc8 100644 --- a/src/model_config_tests/exp_test_helper.py +++ b/src/model_config_tests/exp_test_helper.py @@ -84,30 +84,34 @@ def has_run(self): def setup_reproduce(self): """ - Run payu setup command with --reproduce flag + Run payu setup command and check if manifests files have been changed with `git diff`. """ owd = Path.cwd() # Change to experiment directory and run. os.chdir(self.control_path) try: - setup_command = [ - "payu", - "setup", - "--lab", - str(self.lab_path), - "--reproduce", - ] + setup_command = ["payu", "setup", "--lab", str(self.lab_path)] print(f"Running payu setup command: {setup_command}") - result = sp.run(setup_command, capture_output=True, text=True) + setup_result = sp.run(setup_command, capture_output=True, text=True) + if setup_result.returncode != 0 or "error" in setup_result.stderr.lower(): + raise RuntimeError( + f"Error during payu setup. \n" + f"{'='*10}STDOUT{'='*10}\n {setup_result.stdout}\n" + f"{'='*10}STDERR{'='*10}\n {setup_result.stderr}\n" + ) + result = sp.run( + ["git", "diff", "--name-only", "manifests/"], + capture_output=True, + text=True, + ) finally: # Change back to original working directory os.chdir(owd) - if result.returncode != 0: + if result.stdout != "": raise RuntimeError( - f"Failed to run payu setup with --reproduce. Error: {result.stderr}\n" - f"Full output: {result.stdout}" + f"Manifests have been modified. The modified files include: {result.stdout}.\n" ) def setup_for_test_run(self): From 0d68e9cc9e718ff2e08e03c9138a1c062c0c68c4 Mon Sep 17 00:00:00 2001 From: Qianhui Chen Date: Tue, 17 Mar 2026 14:59:10 +1100 Subject: [PATCH 3/6] Display the top 10 lines in git diff for each modified file in error message. --- .../config_tests/test_bit_reproducibility.py | 2 +- src/model_config_tests/exp_test_helper.py | 19 ++++++++++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/model_config_tests/config_tests/test_bit_reproducibility.py b/src/model_config_tests/config_tests/test_bit_reproducibility.py index 1c69313..49fd476 100644 --- a/src/model_config_tests/config_tests/test_bit_reproducibility.py +++ b/src/model_config_tests/config_tests/test_bit_reproducibility.py @@ -333,7 +333,7 @@ def test_repro_determinism_restart( @pytest.mark.repro_payu_setup def test_repro_payu_setup(control_path, output_path): """ - Test payu setup with --reproduce which errors if payu manifests full hashes are changed + Test payu setup with `git diff` which errors if any files in payu manifests are changed. """ experiment = setup_exp(control_path, output_path, exp_name="repro_payu_setup") try: diff --git a/src/model_config_tests/exp_test_helper.py b/src/model_config_tests/exp_test_helper.py index 50dbdc8..c9ec493 100644 --- a/src/model_config_tests/exp_test_helper.py +++ b/src/model_config_tests/exp_test_helper.py @@ -110,9 +110,22 @@ def setup_reproduce(self): os.chdir(owd) if result.stdout != "": - raise RuntimeError( - f"Manifests have been modified. The modified files include: {result.stdout}.\n" - ) + # Collect and display the top 10 lines of the diff for each modified file + files = result.stdout.strip().split("\n") + error_message = "" + for file in files: + error_message += f"Modifications are detected in {file}:\n" + diff_details = sp.run( + ["git", "diff", f"{file}"], + capture_output=True, + text=True, + ) + diff_lines = diff_details.stdout.splitlines() + top_lines = "\n".join(diff_lines[2:12]) + if len(diff_lines) > 12: + top_lines += "\n... (truncated)" + error_message += f"Changes are: \n {top_lines}\n" + raise RuntimeError(f"{error_message}") def setup_for_test_run(self): """ From 222f1fb2da9f3274f2f1eac7dfb96d06542439c0 Mon Sep 17 00:00:00 2001 From: Qianhui Chen Date: Tue, 24 Mar 2026 10:39:40 +1100 Subject: [PATCH 4/6] Modify the diff message and split handling of exit!=0 and error. --- src/model_config_tests/exp_test_helper.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/model_config_tests/exp_test_helper.py b/src/model_config_tests/exp_test_helper.py index c9ec493..a48114a 100644 --- a/src/model_config_tests/exp_test_helper.py +++ b/src/model_config_tests/exp_test_helper.py @@ -94,12 +94,16 @@ def setup_reproduce(self): setup_command = ["payu", "setup", "--lab", str(self.lab_path)] print(f"Running payu setup command: {setup_command}") setup_result = sp.run(setup_command, capture_output=True, text=True) - if setup_result.returncode != 0 or "error" in setup_result.stderr.lower(): + if setup_result.returncode != 0: raise RuntimeError( f"Error during payu setup. \n" f"{'='*10}STDOUT{'='*10}\n {setup_result.stdout}\n" f"{'='*10}STDERR{'='*10}\n {setup_result.stderr}\n" ) + elif "error" in setup_result.stderr.lower(): + warnings.warn( + f"Payu setup existed safely with errors in stderr:\n{setup_result.stderr}. Proceeding to modification check." + ) result = sp.run( ["git", "diff", "--name-only", "manifests/"], capture_output=True, @@ -112,11 +116,17 @@ def setup_reproduce(self): if result.stdout != "": # Collect and display the top 10 lines of the diff for each modified file files = result.stdout.strip().split("\n") - error_message = "" + error_message = "Modifications are detected in file:\n" + error_message += "\n".join(" - " + file for file in files) + "\n" + error_message += "\nIf md5 hashes have changed, this indicates file contents being different." + error_message += """ +If binhashes/paths have changed but md5's are the same, +this will mean the configuration can reproduce the manifests +but `payu setup` will take longer to run as it needs to re-calculate all the md5 hashes. + """ for file in files: - error_message += f"Modifications are detected in {file}:\n" diff_details = sp.run( - ["git", "diff", f"{file}"], + ["git", "-C", str(self.control_path), "diff", f"{file}"], capture_output=True, text=True, ) @@ -124,7 +134,7 @@ def setup_reproduce(self): top_lines = "\n".join(diff_lines[2:12]) if len(diff_lines) > 12: top_lines += "\n... (truncated)" - error_message += f"Changes are: \n {top_lines}\n" + error_message += f"\n{'='*10} Diff for {file} {'='*10}\n{top_lines}\n" raise RuntimeError(f"{error_message}") def setup_for_test_run(self): From fc3af4b047ce804551c094d4c1adf216ac2fcec7 Mon Sep 17 00:00:00 2001 From: Qianhui Chen Date: Mon, 30 Mar 2026 16:26:03 +1100 Subject: [PATCH 5/6] Split the tests into repro_payu_setup and manifests_unchanged. --- .../config_tests/conftest.py | 8 ++++++ .../config_tests/test_bit_reproducibility.py | 21 ++++++++++++-- src/model_config_tests/exp_test_helper.py | 28 +++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/src/model_config_tests/config_tests/conftest.py b/src/model_config_tests/config_tests/conftest.py index 5fe688d..0c8e3b9 100644 --- a/src/model_config_tests/config_tests/conftest.py +++ b/src/model_config_tests/config_tests/conftest.py @@ -135,6 +135,14 @@ def pytest_configure(config): "markers", "repro_payu_setup: mark tests that check payu setup reproducibility", ) + config.addinivalue_line( + "markers", + "manifests: mark tests that check payu setup does not change manifests files or md5", + ) + config.addinivalue_line( + "markers", + "manifests_unchanged: mark tests that check payu setup does not change manifests files", + ) config.addinivalue_line("markers", "slow: mark tests that are slow to run") config.addinivalue_line( "markers", diff --git a/src/model_config_tests/config_tests/test_bit_reproducibility.py b/src/model_config_tests/config_tests/test_bit_reproducibility.py index 49fd476..ad79fd5 100644 --- a/src/model_config_tests/config_tests/test_bit_reproducibility.py +++ b/src/model_config_tests/config_tests/test_bit_reproducibility.py @@ -330,13 +330,30 @@ def test_repro_determinism_restart( assert produced == expected +@pytest.mark.repro +@pytest.mark.manifests @pytest.mark.repro_payu_setup -def test_repro_payu_setup(control_path, output_path): +def test_payu_setup_repro_flag(control_path, output_path): """ - Test payu setup with `git diff` which errors if any files in payu manifests are changed. + Test payu setup with `--repro` flag which errors if md5 of any files in payu manifests are changed. """ experiment = setup_exp(control_path, output_path, exp_name="repro_payu_setup") try: experiment.setup_reproduce() except Exception as error: pytest.fail(f"{error}") + + +@pytest.mark.manifests +@pytest.mark.manifests_unchanged +def test_manifests_unchanged(control_path, output_path): + """ + Test payu setup with `git diff` which errors if any files in payu manifests are changed. + """ + experiment = setup_exp( + control_path, output_path, exp_name="setup_unchanged_manifests" + ) + try: + experiment.setup_manifests_unchanged() + except Exception as error: + pytest.fail(f"{error}") diff --git a/src/model_config_tests/exp_test_helper.py b/src/model_config_tests/exp_test_helper.py index a48114a..ca6fd64 100644 --- a/src/model_config_tests/exp_test_helper.py +++ b/src/model_config_tests/exp_test_helper.py @@ -83,6 +83,34 @@ def has_run(self): return self.model.output_exists() def setup_reproduce(self): + """ + Run payu setup with `--repro` flag to check if md5 hashes have changed in the manifests. + """ + owd = Path.cwd() + # Change to experiment directory and run. + os.chdir(self.control_path) + + try: + setup_command = [ + "payu", + "setup", + "--lab", + str(self.lab_path), + "--reproduce", + ] + print(f"Running payu setup command: {setup_command}") + result = sp.run(setup_command, capture_output=True, text=True) + finally: + # Change back to original working directory + os.chdir(owd) + + if result.returncode != 0: + raise RuntimeError( + f"Failed to run payu setup with --reproduce. Error: {result.stderr}\n" + f"Full output: {result.stdout}" + ) + + def setup_manifests_unchanged(self): """ Run payu setup command and check if manifests files have been changed with `git diff`. """ From f277c4c15301a2d0ca939e274b0549d0d6ec1a60 Mon Sep 17 00:00:00 2001 From: Qianhui Chen Date: Wed, 1 Apr 2026 10:30:37 +1100 Subject: [PATCH 6/6] Add unit tests on setup_reproduce and setup_manifests_unchanged. --- src/model_config_tests/exp_test_helper.py | 5 +- tests/test_exp_test_helper.py | 73 ++++++++++++++++++++++- 2 files changed, 73 insertions(+), 5 deletions(-) diff --git a/src/model_config_tests/exp_test_helper.py b/src/model_config_tests/exp_test_helper.py index ca6fd64..51ddc28 100644 --- a/src/model_config_tests/exp_test_helper.py +++ b/src/model_config_tests/exp_test_helper.py @@ -128,10 +128,7 @@ def setup_manifests_unchanged(self): f"{'='*10}STDOUT{'='*10}\n {setup_result.stdout}\n" f"{'='*10}STDERR{'='*10}\n {setup_result.stderr}\n" ) - elif "error" in setup_result.stderr.lower(): - warnings.warn( - f"Payu setup existed safely with errors in stderr:\n{setup_result.stderr}. Proceeding to modification check." - ) + result = sp.run( ["git", "diff", "--name-only", "manifests/"], capture_output=True, diff --git a/tests/test_exp_test_helper.py b/tests/test_exp_test_helper.py index 7ffc633..0a7417e 100644 --- a/tests/test_exp_test_helper.py +++ b/tests/test_exp_test_helper.py @@ -1,7 +1,7 @@ import shutil import subprocess from pathlib import Path -from unittest.mock import Mock, patch +from unittest.mock import MagicMock, Mock, patch import pytest import yaml @@ -556,3 +556,74 @@ def test_experiments_check_experiment_error(tmp_path): ) with pytest.raises(RuntimeError, match=error_msg): exps.check_experiment("error_exp") + + +@patch("subprocess.run") +def test_setup_reproduce_error(mock_run, exp): + """Test that payu setup --repro fails raises an error and return to original work directory""" + # Mock the payu setup --repro to fail + mock_result = MagicMock() + mock_result.returncode = 1 + mock_result.stderr = "MD5 mismatch" + mock_result.stdout = "Check manifest" + mock_run.return_value = mock_result + + with pytest.raises(RuntimeError) as excinfo: + exp.setup_reproduce() + + assert ( + f"Failed to run payu setup with --reproduce. Error: {mock_result.stderr}" + in str(excinfo.value) + ) + assert f"Full output: {mock_result.stdout}" in str(excinfo.value) + + # assert returning to the original work directory + assert Path.cwd() == exp.control_path + + +@patch("subprocess.run") +def test_setup_manifests_unchanged_fail_setup(mock_run, exp): + """Test that an error is raised when payu setup fails in setup_manifests_unchanged()""" + # Mock the payu setup --repro to fail with unchanged manifests + mock_result = MagicMock() + mock_result.returncode = 1 + mock_result.stderr = "Setup failed" + mock_result.stdout = "Payu setup output" + mock_run.return_value = mock_result + + with pytest.raises(RuntimeError) as excinfo: + exp.setup_manifests_unchanged() + + assert "Error during payu setup." in str(excinfo.value) + assert f"{'='*10}STDOUT{'='*10}\n {mock_result.stdout}\n" in str(excinfo.value) + + # assert returning to the original work directory + assert Path.cwd() == exp.control_path + + +@patch("subprocess.run") +def test_setup_manifests_unchanged_show_changes(mock_run, exp): + """Test that when manifests are changed, the `git diff` results are printed to stdout""" + # Mock the `payu setup` succeed first + setup_success = MagicMock(returncode=0, stdout="Payu setup succeeded") + + # Then mock the `git diff --name-only` to show which files are changed + git_diff_name_only = MagicMock(returncode=1, stdout="manifests/input.yaml") + + # Mock the `git diff` to show the detailed changes in the file + git_diff_output = ( + "--- a/manifests/input.yaml\n+++ b/manifests/input.yaml\n+new line\n-old line" + ) + git_diff_run = MagicMock(returncode=0, stdout=git_diff_output) + + # Run these mocks in sequence + mock_run.side_effect = [setup_success, git_diff_name_only, git_diff_run] + + with pytest.raises(RuntimeError) as excinfo: + exp.setup_manifests_unchanged() + + assert "Modifications are detected in file:\n" in str(excinfo.value) + assert git_diff_output in str(excinfo.value) + + # assert returning to the original work directory + assert Path.cwd() == exp.control_path