From c14be7d6af5b9e49bf2073ae4979976234ffe4b9 Mon Sep 17 00:00:00 2001 From: Matthew Liu Date: Fri, 31 Jul 2026 16:21:48 +0900 Subject: [PATCH 1/3] Initial implementation --- RUFAS/e2e_test_results_handler.py | 280 +++++++++++++++++- RUFAS/input/metadata/properties/default.json | 20 ++ .../animals_only_must_change_variables.json | 4 + .../end_to_end_testing_result_paths.json | 36 ++- .../field_and_feed_must_change_variables.json | 4 + .../freestall_must_change_variables.json | 4 + .../open_lot_must_change_variables.json | 4 + tests/test_e2e_test_results_handler.py | 271 ++++++++++++++++- 8 files changed, 604 insertions(+), 19 deletions(-) create mode 100644 input/data/end_to_end_testing/animals_only_must_change_variables.json create mode 100644 input/data/end_to_end_testing/field_and_feed_must_change_variables.json create mode 100644 input/data/end_to_end_testing/freestall_must_change_variables.json create mode 100644 input/data/end_to_end_testing/open_lot_must_change_variables.json diff --git a/RUFAS/e2e_test_results_handler.py b/RUFAS/e2e_test_results_handler.py index f85f544c72..6e0184f6de 100644 --- a/RUFAS/e2e_test_results_handler.py +++ b/RUFAS/e2e_test_results_handler.py @@ -1,4 +1,5 @@ import json +import re from collections import namedtuple from pathlib import Path import shutil @@ -13,8 +14,14 @@ from RUFAS.units import MeasurementUnits from RUFAS.util import Utility -ResultPathType = namedtuple("ResultPathType", ["domain", "expected_results_path", "actual_results_path", "tolerance"]) +ResultPathType = namedtuple( + "ResultPathType", + ["domain", "expected_results_path", "actual_results_path", "tolerance", "must_change_variables_path"], + defaults=[""], +) ORDERED_EXPECTED_RESULTS_FILE_KEYS = ["name", "filters", "expected_results_last_updated", "expected_results"] +MUST_CHANGE_VARIABLES_KEY = "must_change_variables" +TOP_LEVEL_DIFF_PATH_PATTERN = re.compile(r"^root\['([^']+)'\]") class E2ETestResultsHandler: @@ -36,6 +43,14 @@ def compare_actual_and_expected_test_results( variable names in the actual results. output_prefix : str The output prefix for the current e2e run. + + Notes + ----- + Variables flagged in the input set's must-change variables file are held to the opposite assertion of the + regular comparison: each flagged variable must differ from its recorded expected value beyond the domain + tolerance, and its differences are not reported as regular failures. The comparison results additionally + report ``changed_variables``, the names of the unflagged variables whose values differ from the expected + results, so a subject matter expert can evaluate them and flag the ones that are expected to change. """ om = OutputManager() info_map: dict[str, Any] = { @@ -43,6 +58,9 @@ def compare_actual_and_expected_test_results( "function": E2ETestResultsHandler.compare_actual_and_expected_test_results.__name__, } test_result_path_sets = E2ETestResultsHandler._get_test_result_paths(output_prefix) + must_change_variables = E2ETestResultsHandler._load_must_change_variables(test_result_path_sets) + matched_must_change_variables: set[str] = set() + all_domains_compared: bool = True for path_set in test_result_path_sets: info_map["domain"] = path_set.domain @@ -60,6 +78,7 @@ def compare_actual_and_expected_test_results( "Could not find actual end-to-end testing results", info_map, ) + all_domains_compared = False continue with open(path_to_actual_results, "r", encoding="utf-8") as results: actual_results = json.load(results) @@ -71,9 +90,20 @@ def compare_actual_and_expected_test_results( expected_results=expected_results, conversion_csv_path=Path(convert_variable_table_path) ) - diff = DeepDiff(expected_results, actual_results, ignore_order=True, verbose_level=2, significant_digits=3) + domain_must_change_variables = sorted(name for name in must_change_variables if name in expected_results) + matched_must_change_variables.update(domain_must_change_variables) + comparison_expected = {k: v for k, v in expected_results.items() if k not in must_change_variables} + comparison_actual = {k: v for k, v in actual_results.items() if k not in must_change_variables} + + diff = DeepDiff( + comparison_expected, comparison_actual, ignore_order=True, verbose_level=2, significant_digits=3 + ) filtered_diff = E2ETestResultsHandler.filter_insignificant_changes(diff, path_set.tolerance) + changed_variables = E2ETestResultsHandler._extract_changed_variable_names(filtered_diff) + must_change_satisfied, must_change_violations = E2ETestResultsHandler._evaluate_must_change_variables( + expected_results, actual_results, domain_must_change_variables, path_set.tolerance + ) is_difference_in_results: bool = False if (filtered_diff == {}) else True if is_difference_in_results: @@ -82,18 +112,41 @@ def compare_actual_and_expected_test_results( "Identified differences between actual and expected results.", info_map, ) - else: + if must_change_violations: + om.add_error( + f"End-to-end testing failed for {path_set.domain}", + f"Must-change variables did not change: {sorted(must_change_violations)}", + info_map, + ) + if not is_difference_in_results and not must_change_violations: om.add_log( f"End-to-end testing succeeded for {path_set.domain}", "No differences found between actual and expected end-to-end testing results.", info_map, ) - end_to_end_testing_passing: bool = not is_difference_in_results - filtered_diff.update({"end_to_end_testing_passing": end_to_end_testing_passing}) + end_to_end_testing_passing: bool = not is_difference_in_results and not must_change_violations + comparison_results: dict[str, Any] = dict(filtered_diff) + if changed_variables: + comparison_results["changed_variables"] = changed_variables + if domain_must_change_variables: + comparison_results["must_change_satisfied"] = must_change_satisfied + comparison_results["must_change_violations"] = must_change_violations + comparison_results["end_to_end_testing_passing"] = end_to_end_testing_passing info_map.update({"units": MeasurementUnits.UNITLESS, "prefix": path_set.domain}) - for comparison_type, difference in filtered_diff.items(): + for comparison_type, difference in comparison_results.items(): om.add_variable(comparison_type, difference, info_map) + unknown_must_change_variables = must_change_variables - matched_must_change_variables + if unknown_must_change_variables and all_domains_compared: + info_map.pop("domain", None) + info_map.pop("prefix", None) + om.add_error( + "End-to-end testing must-change configuration error", + "Must-change variables not found in the expected results of any domain: " + f"{sorted(unknown_must_change_variables)}", + info_map, + ) + @staticmethod def _convert_expected_result_variable_names( expected_results: dict[str, Any], conversion_csv_path: Path @@ -271,10 +324,218 @@ def _get_test_result_paths(output_prefix: str) -> list[ResultPathType]: path_set["expected_results_path"], path_set["actual_results_path"], path_set["tolerance"], + path_set.get("must_change_variables_path", ""), ) ) return test_result_paths + @staticmethod + def _load_must_change_variables(test_result_path_sets: list[ResultPathType]) -> set[str]: + """ + Loads the names of the variables flagged as must change for an end-to-end testing input set. + + Parameters + ---------- + test_result_path_sets : list[ResultPathType] + List of result path sets for the input set, each optionally referencing a must-change variables file + through its ``must_change_variables_path`` field. + + Returns + ------- + set[str] + The union of the variable names listed in the referenced must-change variables files. Path sets with an + empty ``must_change_variables_path`` are skipped. + + Raises + ------ + FileNotFoundError + If a referenced must-change variables file does not exist. + ValueError + If a referenced file is not valid JSON, or does not contain a list of strings under the + ``must_change_variables`` key. + """ + om = OutputManager() + info_map: dict[str, Any] = { + "class": E2ETestResultsHandler.__class__.__name__, + "function": E2ETestResultsHandler._load_must_change_variables.__name__, + } + must_change_variables: set[str] = set() + must_change_paths = { + path_set.must_change_variables_path + for path_set in test_result_path_sets + if path_set.must_change_variables_path + } + for path_str in sorted(must_change_paths): + path = Path(path_str) + if not path.exists(): + om.add_error( + "End-to-end testing must-change configuration error", + f"Must-change variables file not found: {path}", + info_map, + ) + raise FileNotFoundError(f"E2E testing error: Must-change variables file not found: {path}") + try: + with open(path, "r", encoding="utf-8") as must_change_file: + file_contents = json.load(must_change_file) + except json.JSONDecodeError as e: + om.add_error( + "End-to-end testing must-change configuration error", + f"Must-change variables file {path} is not valid JSON: {e}", + info_map, + ) + raise ValueError(f"E2E testing error: Must-change variables file {path} is not valid JSON.") from e + variable_names = file_contents.get(MUST_CHANGE_VARIABLES_KEY) if isinstance(file_contents, dict) else None + if not isinstance(variable_names, list) or not all(isinstance(name, str) for name in variable_names): + om.add_error( + "End-to-end testing must-change configuration error", + f"Must-change variables file {path} must contain a list of variable names under the " + f"'{MUST_CHANGE_VARIABLES_KEY}' key.", + info_map, + ) + raise ValueError( + f"E2E testing error: Must-change variables file {path} must contain a list of variable names " + f"under the '{MUST_CHANGE_VARIABLES_KEY}' key." + ) + must_change_variables.update(variable_names) + return must_change_variables + + @staticmethod + def _evaluate_must_change_variables( + expected_results: dict[str, Any], + actual_results: dict[str, Any], + must_change_variable_names: list[str], + tolerance: float, + ) -> tuple[list[str], dict[str, str]]: + """ + Checks that each variable flagged as must change actually differs from its recorded expected value. + + Parameters + ---------- + expected_results : dict[str, Any] + The expected results for a domain, keyed by variable name. + actual_results : dict[str, Any] + The actual results for a domain, keyed by variable name. + must_change_variable_names : list[str] + The must-change variable names present in ``expected_results``. + tolerance : float + The threshold (expressed as a percent) below which a difference is considered no change. + + Returns + ------- + tuple[list[str], dict[str, str]] + A list of the must-change variables whose values differ from the expected results, and a dictionary + mapping each violating must-change variable to the reason it failed: either its value still matches the + expected results, or it is missing from the actual results. + """ + must_change_satisfied: list[str] = [] + must_change_violations: dict[str, str] = {} + for variable_name in must_change_variable_names: + if variable_name not in actual_results: + must_change_violations[variable_name] = ( + "Flagged as must change but the variable is missing from the actual results." + ) + continue + pair_diff = DeepDiff( + {variable_name: expected_results[variable_name]}, + {variable_name: actual_results[variable_name]}, + ignore_order=True, + verbose_level=2, + significant_digits=3, + ) + filtered_pair_diff = E2ETestResultsHandler.filter_insignificant_changes(pair_diff, tolerance) + if filtered_pair_diff == {}: + must_change_violations[variable_name] = ( + "Flagged as must change but the value still matches the expected results within the tolerance." + ) + else: + must_change_satisfied.append(variable_name) + return must_change_satisfied, must_change_violations + + @staticmethod + def _extract_changed_variable_names(diff_result: dict[str, Any]) -> list[str]: + """ + Compiles the names of the variables that a ``DeepDiff`` result reports as different. + + Parameters + ---------- + diff_result : dict[str, Any] + A ``DeepDiff`` result mapping change categories (e.g. ``values_changed``) to changed paths. + + Returns + ------- + list[str] + The sorted, deduplicated top-level variable names extracted from the changed paths. Paths that do not + start with a top-level dictionary key (e.g. a change to the results root) are skipped. + """ + changed_variable_names: set[str] = set() + for changed_entries in diff_result.values(): + if isinstance(changed_entries, dict): + changed_paths = list(changed_entries.keys()) + elif isinstance(changed_entries, (list, set, tuple)): + changed_paths = list(changed_entries) + else: + continue + for changed_path in changed_paths: + match = TOP_LEVEL_DIFF_PATH_PATTERN.match(str(changed_path)) + if match: + changed_variable_names.add(match.group(1)) + return sorted(changed_variable_names) + + @staticmethod + def _reset_must_change_variables(test_result_path_sets: list[ResultPathType]) -> None: + """ + Empties the must-change variables files referenced by the given result path sets. + + Parameters + ---------- + test_result_path_sets : list[ResultPathType] + List of result path sets for the input set, each optionally referencing a must-change variables file + through its ``must_change_variables_path`` field. + + Notes + ----- + Called after the expected results are regenerated: at that point the recorded expected values match the + actual values, so the must-change flags are stale by definition and would fail the next comparison run. + Files that are missing, unparsable, or already empty are left untouched. + """ + om = OutputManager() + info_map: dict[str, Any] = { + "class": E2ETestResultsHandler.__class__.__name__, + "function": E2ETestResultsHandler._reset_must_change_variables.__name__, + } + must_change_paths = { + path_set.must_change_variables_path + for path_set in test_result_path_sets + if path_set.must_change_variables_path + } + for path_str in sorted(must_change_paths): + path = Path(path_str) + if not path.exists(): + continue + try: + with open(path, "r", encoding="utf-8") as must_change_file: + file_contents = json.load(must_change_file) + except json.JSONDecodeError as e: + om.add_warning( + "End-to-end testing must-change variables not cleared", + f"Must-change variables file {path} is not valid JSON and was not cleared: {e}", + info_map, + ) + continue + variable_names = file_contents.get(MUST_CHANGE_VARIABLES_KEY) if isinstance(file_contents, dict) else None + if not variable_names: + continue + file_contents[MUST_CHANGE_VARIABLES_KEY] = [] + with open(path, "w", encoding="utf-8") as must_change_file: + json.dump(file_contents, must_change_file, indent=4) + must_change_file.write("\n") + om.add_warning( + "End-to-end testing must-change variables cleared", + f"Cleared must-change variables {variable_names} from {path} after regenerating the expected " + "results. Re-add any variables that are still expected to change.", + info_map, + ) + @staticmethod def is_significant(changes: dict[str, Any], tolerance: float) -> bool: """ @@ -393,6 +654,12 @@ def update_expected_test_results(output_dir: Path, output_prefix: str) -> None: The directory to which the actual results are written to. output_prefix : str The prefix to give the output file names. + + Notes + ----- + After the expected results are regenerated, the input set's must-change variables file is emptied: the + recorded expected values then match the actual values, so any remaining must-change flags would fail the + next comparison run. """ om = OutputManager() info_map: dict[str, Any] = { @@ -467,6 +734,7 @@ def update_expected_test_results(output_dir: Path, output_prefix: str) -> None: finally: if backup_path.exists(): backup_path.unlink() + E2ETestResultsHandler._reset_must_change_variables(test_result_path_sets) @staticmethod def _get_matching_path(dir_path: Path, path_set: ResultPathType) -> Path | None: diff --git a/RUFAS/input/metadata/properties/default.json b/RUFAS/input/metadata/properties/default.json index 7432add8e1..1a0f7f6ebd 100644 --- a/RUFAS/input/metadata/properties/default.json +++ b/RUFAS/input/metadata/properties/default.json @@ -6349,6 +6349,11 @@ "description": "The relative tolerance (expressed as a percent) used to compare expected and actual results for changed values found in the e2e results comparison.", "default": 0.0, "minimum": 0.0 + }, + "must_change_variables_path": { + "type": "string", + "description": "Path to the file listing the variables that are expected to differ from the recorded expected results for this input set. Each listed variable must differ from its expected value beyond the domain tolerance for the end-to-end test to pass.", + "default": "" } } }, @@ -6373,6 +6378,11 @@ "description": "The relative tolerance (expressed as a percent) used to compare expected and actual results for changed values found in the e2e results comparison.", "default": 0.0, "minimum": 0.0 + }, + "must_change_variables_path": { + "type": "string", + "description": "Path to the file listing the variables that are expected to differ from the recorded expected results for this input set. Each listed variable must differ from its expected value beyond the domain tolerance for the end-to-end test to pass.", + "default": "" } } }, @@ -6397,6 +6407,11 @@ "description": "The relative tolerance (expressed as a percent) used to compare expected and actual results for changed values found in the e2e results comparison.", "default": 0.0, "minimum": 0.0 + }, + "must_change_variables_path": { + "type": "string", + "description": "Path to the file listing the variables that are expected to differ from the recorded expected results for this input set. Each listed variable must differ from its expected value beyond the domain tolerance for the end-to-end test to pass.", + "default": "" } } }, @@ -6421,6 +6436,11 @@ "description": "The relative tolerance (expressed as a percent) used to compare expected and actual results for changed values found in the e2e results comparison.", "default": 0.0, "minimum": 0.0 + }, + "must_change_variables_path": { + "type": "string", + "description": "Path to the file listing the variables that are expected to differ from the recorded expected results for this input set. Each listed variable must differ from its expected value beyond the domain tolerance for the end-to-end test to pass.", + "default": "" } } } diff --git a/input/data/end_to_end_testing/animals_only_must_change_variables.json b/input/data/end_to_end_testing/animals_only_must_change_variables.json new file mode 100644 index 0000000000..945a3cb028 --- /dev/null +++ b/input/data/end_to_end_testing/animals_only_must_change_variables.json @@ -0,0 +1,4 @@ +{ + "description": "Variables listed in 'must_change_variables' are expected to differ from the recorded expected results, e.g. because of a known model change whose new values are not yet known. During an end-to-end testing run, each listed variable must differ from its recorded expected value beyond the domain tolerance: a matching value is reported as a failure, and differences in listed variables are not. Variable names must exactly match keys in the 'expected_results' of this input set's e2e_json_*_filter.json files; the 'changed_variables' list in the end-to-end comparison results shows which variables currently differ. Running the UPDATE_E2E_TEST_RESULTS task empties this list after regenerating the expected results.", + "must_change_variables": [] +} diff --git a/input/data/end_to_end_testing/end_to_end_testing_result_paths.json b/input/data/end_to_end_testing/end_to_end_testing_result_paths.json index 7a3cc6af61..3c7cb87ca8 100644 --- a/input/data/end_to_end_testing/end_to_end_testing_result_paths.json +++ b/input/data/end_to_end_testing/end_to_end_testing_result_paths.json @@ -5,25 +5,29 @@ "domain": "CropAndSoil", "expected_results_path": "input/data/end_to_end_testing/freestall/e2e_json_crop_soil_filter.json", "actual_results_path": "freestall_e2e_saved_variables_e2e_crop_and_soil_", - "tolerance": 0.1 + "tolerance": 0.1, + "must_change_variables_path": "input/data/end_to_end_testing/freestall_must_change_variables.json" }, { "domain": "Animal", "expected_results_path": "input/data/end_to_end_testing/freestall/e2e_json_animal_filter.json", "actual_results_path": "freestall_e2e_saved_variables_e2e_animal_", - "tolerance": 0.1 + "tolerance": 0.1, + "must_change_variables_path": "input/data/end_to_end_testing/freestall_must_change_variables.json" }, { "domain": "Manure", "expected_results_path": "input/data/end_to_end_testing/freestall/e2e_json_manure_filter.json", "actual_results_path": "freestall_e2e_saved_variables_e2e_manure_", - "tolerance": 0.1 + "tolerance": 0.1, + "must_change_variables_path": "input/data/end_to_end_testing/freestall_must_change_variables.json" }, { "domain": "Feed", "expected_results_path": "input/data/end_to_end_testing/freestall/e2e_json_feed_filter.json", "actual_results_path": "freestall_e2e_saved_variables_e2e_feed_", - "tolerance": 0.1 + "tolerance": 0.1, + "must_change_variables_path": "input/data/end_to_end_testing/freestall_must_change_variables.json" } ], "open_lot_e2e": [ @@ -31,25 +35,29 @@ "domain": "CropAndSoil", "expected_results_path": "input/data/end_to_end_testing/open_lot/e2e_json_crop_soil_filter.json", "actual_results_path": "open_lot_e2e_saved_variables_e2e_crop_and_soil_", - "tolerance": 0.1 + "tolerance": 0.1, + "must_change_variables_path": "input/data/end_to_end_testing/open_lot_must_change_variables.json" }, { "domain": "Animal", "expected_results_path": "input/data/end_to_end_testing/open_lot/e2e_json_animal_filter.json", "actual_results_path": "open_lot_e2e_saved_variables_e2e_animal_", - "tolerance": 0.1 + "tolerance": 0.1, + "must_change_variables_path": "input/data/end_to_end_testing/open_lot_must_change_variables.json" }, { "domain": "Manure", "expected_results_path": "input/data/end_to_end_testing/open_lot/e2e_json_manure_filter.json", "actual_results_path": "open_lot_e2e_saved_variables_e2e_manure_", - "tolerance": 0.1 + "tolerance": 0.1, + "must_change_variables_path": "input/data/end_to_end_testing/open_lot_must_change_variables.json" }, { "domain": "Feed", "expected_results_path": "input/data/end_to_end_testing/open_lot/e2e_json_feed_filter.json", "actual_results_path": "open_lot_e2e_saved_variables_e2e_feed_", - "tolerance": 0.1 + "tolerance": 0.1, + "must_change_variables_path": "input/data/end_to_end_testing/open_lot_must_change_variables.json" } ], "field_and_feed_e2e": [ @@ -57,19 +65,22 @@ "domain": "CropAndSoil", "expected_results_path": "input/data/end_to_end_testing/field_and_feed/e2e_json_crop_soil_filter.json", "actual_results_path": "field_and_feed_e2e_saved_variables_e2e_crop_and_soil_", - "tolerance": 0.1 + "tolerance": 0.1, + "must_change_variables_path": "input/data/end_to_end_testing/field_and_feed_must_change_variables.json" }, { "domain": "Manure", "expected_results_path": "input/data/end_to_end_testing/field_and_feed/e2e_json_manure_filter.json", "actual_results_path": "field_and_feed_e2e_saved_variables_e2e_manure_", - "tolerance": 0.1 + "tolerance": 0.1, + "must_change_variables_path": "input/data/end_to_end_testing/field_and_feed_must_change_variables.json" }, { "domain": "Feed", "expected_results_path": "input/data/end_to_end_testing/field_and_feed/e2e_json_feed_filter.json", "actual_results_path": "field_and_feed_e2e_saved_variables_e2e_feed_", - "tolerance": 0.1 + "tolerance": 0.1, + "must_change_variables_path": "input/data/end_to_end_testing/field_and_feed_must_change_variables.json" } ], "animals_only_e2e": [ @@ -77,7 +88,8 @@ "domain": "Animal", "expected_results_path": "input/data/end_to_end_testing/animals_only/e2e_json_animal_filter.json", "actual_results_path": "animals_only_e2e_saved_variables_e2e_animal_", - "tolerance": 0.1 + "tolerance": 0.1, + "must_change_variables_path": "input/data/end_to_end_testing/animals_only_must_change_variables.json" } ] } diff --git a/input/data/end_to_end_testing/field_and_feed_must_change_variables.json b/input/data/end_to_end_testing/field_and_feed_must_change_variables.json new file mode 100644 index 0000000000..945a3cb028 --- /dev/null +++ b/input/data/end_to_end_testing/field_and_feed_must_change_variables.json @@ -0,0 +1,4 @@ +{ + "description": "Variables listed in 'must_change_variables' are expected to differ from the recorded expected results, e.g. because of a known model change whose new values are not yet known. During an end-to-end testing run, each listed variable must differ from its recorded expected value beyond the domain tolerance: a matching value is reported as a failure, and differences in listed variables are not. Variable names must exactly match keys in the 'expected_results' of this input set's e2e_json_*_filter.json files; the 'changed_variables' list in the end-to-end comparison results shows which variables currently differ. Running the UPDATE_E2E_TEST_RESULTS task empties this list after regenerating the expected results.", + "must_change_variables": [] +} diff --git a/input/data/end_to_end_testing/freestall_must_change_variables.json b/input/data/end_to_end_testing/freestall_must_change_variables.json new file mode 100644 index 0000000000..945a3cb028 --- /dev/null +++ b/input/data/end_to_end_testing/freestall_must_change_variables.json @@ -0,0 +1,4 @@ +{ + "description": "Variables listed in 'must_change_variables' are expected to differ from the recorded expected results, e.g. because of a known model change whose new values are not yet known. During an end-to-end testing run, each listed variable must differ from its recorded expected value beyond the domain tolerance: a matching value is reported as a failure, and differences in listed variables are not. Variable names must exactly match keys in the 'expected_results' of this input set's e2e_json_*_filter.json files; the 'changed_variables' list in the end-to-end comparison results shows which variables currently differ. Running the UPDATE_E2E_TEST_RESULTS task empties this list after regenerating the expected results.", + "must_change_variables": [] +} diff --git a/input/data/end_to_end_testing/open_lot_must_change_variables.json b/input/data/end_to_end_testing/open_lot_must_change_variables.json new file mode 100644 index 0000000000..945a3cb028 --- /dev/null +++ b/input/data/end_to_end_testing/open_lot_must_change_variables.json @@ -0,0 +1,4 @@ +{ + "description": "Variables listed in 'must_change_variables' are expected to differ from the recorded expected results, e.g. because of a known model change whose new values are not yet known. During an end-to-end testing run, each listed variable must differ from its recorded expected value beyond the domain tolerance: a matching value is reported as a failure, and differences in listed variables are not. Variable names must exactly match keys in the 'expected_results' of this input set's e2e_json_*_filter.json files; the 'changed_variables' list in the end-to-end comparison results shows which variables currently differ. Running the UPDATE_E2E_TEST_RESULTS task empties this list after regenerating the expected results.", + "must_change_variables": [] +} diff --git a/tests/test_e2e_test_results_handler.py b/tests/test_e2e_test_results_handler.py index f030658c76..a8966350c7 100644 --- a/tests/test_e2e_test_results_handler.py +++ b/tests/test_e2e_test_results_handler.py @@ -6,7 +6,16 @@ import pytest from pytest_mock import MockerFixture -from RUFAS.e2e_test_results_handler import E2ETestResultsHandler, ResultPathType +from RUFAS.e2e_test_results_handler import E2ETestResultsHandler, MUST_CHANGE_VARIABLES_KEY, ResultPathType + + +def write_must_change_file(path: Path, contents: Any) -> None: + """Writes a must-change variables file used by the must-change tests.""" + with open(path, "w", encoding="utf-8") as file: + if isinstance(contents, str): + file.write(contents) + else: + json.dump(contents, file) @pytest.mark.parametrize( @@ -40,6 +49,7 @@ def test_compare_simulation_outputs_to_expected_outputs( add_log = mocker.patch("RUFAS.e2e_test_results_handler.OutputManager.add_log") add_error = mocker.patch("RUFAS.e2e_test_results_handler.OutputManager.add_error") add_var = mocker.patch("RUFAS.e2e_test_results_handler.OutputManager.add_variable") + mocker.patch.object(E2ETestResultsHandler, "_load_must_change_variables", return_value=set()) mock_convert_variable_name = mocker.patch( "RUFAS.e2e_test_results_handler.E2ETestResultsHandler._convert_expected_result_variable_names" ) @@ -517,6 +527,7 @@ def test_update_expected_test_results( get_result_paths = mocker.patch.object(E2ETestResultsHandler, "_get_test_result_paths", return_value=[results_path]) mocker.patch.object(E2ETestResultsHandler, "_get_matching_path", return_value=matching_path) + mock_reset_must_change = mocker.patch.object(E2ETestResultsHandler, "_reset_must_change_variables") if matching_path: mock_open = mocker.patch("builtins.open", mocker.mock_open(read_data='{"expected_results": {}}')) @@ -546,6 +557,7 @@ def test_update_expected_test_results( # Assert get_result_paths.assert_called_once() + assert mock_reset_must_change.call_count == (0 if raise_exception else 1) if matching_path: if raise_exception: @@ -654,3 +666,260 @@ def test_write_formatted_json(data: dict[str, dict[str, str]], should_raise: boo assert "expected_results_last_updated" in parsed_json expected_results_str = json.dumps(data["expected_results"], separators=(",", ":")) assert written_data.count(expected_results_str) == 1 + + +def make_result_path_set(must_change_variables_path: str) -> ResultPathType: + """Returns a ResultPathType with dummy paths and the given must-change variables path.""" + return ResultPathType("domain", "expected", "actual_", 0.1, must_change_variables_path) + + +def test_load_must_change_variables(mocker: MockerFixture, tmp_path: Path) -> None: + """Tests that _load_must_change_variables unions the files referenced by the path sets.""" + mocker.patch("RUFAS.e2e_test_results_handler.OutputManager.__init__", return_value=None) + add_error = mocker.patch("RUFAS.e2e_test_results_handler.OutputManager.add_error") + file_one = tmp_path / "must_change_one.json" + file_two = tmp_path / "must_change_two.json" + write_must_change_file(file_one, {"description": "ignored", MUST_CHANGE_VARIABLES_KEY: ["A.x", "A.y"]}) + write_must_change_file(file_two, {MUST_CHANGE_VARIABLES_KEY: ["A.y", "B.z"]}) + path_sets = [ + make_result_path_set(str(file_one)), + make_result_path_set(str(file_one)), + make_result_path_set(str(file_two)), + make_result_path_set(""), + ] + + result = E2ETestResultsHandler._load_must_change_variables(path_sets) + + assert result == {"A.x", "A.y", "B.z"} + add_error.assert_not_called() + + +def test_load_must_change_variables_without_configured_paths(mocker: MockerFixture) -> None: + """Tests that _load_must_change_variables returns an empty set when no paths are configured.""" + mocker.patch("RUFAS.e2e_test_results_handler.OutputManager.__init__", return_value=None) + + assert E2ETestResultsHandler._load_must_change_variables([make_result_path_set("")]) == set() + + +def test_load_must_change_variables_missing_file(mocker: MockerFixture, tmp_path: Path) -> None: + """Tests that _load_must_change_variables raises when a referenced file does not exist.""" + mocker.patch("RUFAS.e2e_test_results_handler.OutputManager.__init__", return_value=None) + add_error = mocker.patch("RUFAS.e2e_test_results_handler.OutputManager.add_error") + path_sets = [make_result_path_set(str(tmp_path / "no_such_file.json"))] + + with pytest.raises(FileNotFoundError): + E2ETestResultsHandler._load_must_change_variables(path_sets) + add_error.assert_called_once() + + +@pytest.mark.parametrize( + "file_contents", + [ + "{not valid json", + ["A.x"], + {"wrong_key": ["A.x"]}, + {MUST_CHANGE_VARIABLES_KEY: "A.x"}, + {MUST_CHANGE_VARIABLES_KEY: ["A.x", 3]}, + ], +) +def test_load_must_change_variables_invalid_contents(mocker: MockerFixture, tmp_path: Path, file_contents: Any) -> None: + """Tests that _load_must_change_variables raises for unparsable or malformed files.""" + mocker.patch("RUFAS.e2e_test_results_handler.OutputManager.__init__", return_value=None) + add_error = mocker.patch("RUFAS.e2e_test_results_handler.OutputManager.add_error") + file_path = tmp_path / "must_change.json" + write_must_change_file(file_path, file_contents) + + with pytest.raises(ValueError): + E2ETestResultsHandler._load_must_change_variables([make_result_path_set(str(file_path))]) + add_error.assert_called_once() + + +@pytest.mark.parametrize( + "expected_value, actual_value, tolerance, expect_satisfied", + [ + # Change well beyond the tolerance + ({"values": [1.0]}, {"values": [5.0]}, 0.1, True), + # Identical values + ({"values": [1.0]}, {"values": [1.0]}, 0.1, False), + # Change within the tolerance counts as no change + ({"values": [100.0]}, {"values": [100.5]}, 1.0, False), + # Non-numerical change + ({"values": [["Holstein"]]}, {"values": [["Jersey"]]}, 0.1, True), + # Structural change + ( + {"values": [{"field_name": "field_1"}]}, + {"values": [{"field_name": "field_1"}, {"field_name": "f2"}]}, + 0.1, + True, + ), + ], +) +def test_evaluate_must_change_variables( + expected_value: dict[str, Any], actual_value: dict[str, Any], tolerance: float, expect_satisfied: bool +) -> None: + """Tests _evaluate_must_change_variables against real DeepDiff comparisons.""" + expected_results = {"A.x": expected_value, "A.y": {"values": [2.0]}} + actual_results = {"A.x": actual_value, "A.y": {"values": [2.0]}} + + satisfied, violations = E2ETestResultsHandler._evaluate_must_change_variables( + expected_results, actual_results, ["A.x"], tolerance + ) + + if expect_satisfied: + assert satisfied == ["A.x"] + assert violations == {} + else: + assert satisfied == [] + assert list(violations.keys()) == ["A.x"] + + +def test_evaluate_must_change_variables_missing_from_actual() -> None: + """Tests that a must-change variable missing from the actual results is reported as a violation.""" + satisfied, violations = E2ETestResultsHandler._evaluate_must_change_variables( + {"A.x": {"values": [1.0]}}, {}, ["A.x"], 0.1 + ) + + assert satisfied == [] + assert list(violations.keys()) == ["A.x"] + assert "missing" in violations["A.x"] + + +@pytest.mark.parametrize( + "diff_result, expected_names", + [ + ({}, []), + ( + { + "values_changed": { + "root['A.x']['values'][0]": {"old_value": 1.0, "new_value": 2.0}, + "root['A.y']['values'][3]": {"old_value": 1.0, "new_value": 2.0}, + "root['A.x']['values'][7]": {"old_value": 3.0, "new_value": 4.0}, + } + }, + ["A.x", "A.y"], + ), + ( + { + "dictionary_item_added": {"root['B.z']": {"values": [1.0]}}, + "dictionary_item_removed": ["root['C.w']", "root"], + }, + ["B.z", "C.w"], + ), + ({"end_to_end_testing_passing": True}, []), + ], +) +def test_extract_changed_variable_names(diff_result: dict[str, Any], expected_names: list[str]) -> None: + """Tests _extract_changed_variable_names across DeepDiff change categories.""" + assert E2ETestResultsHandler._extract_changed_variable_names(diff_result) == expected_names + + +def test_reset_must_change_variables(mocker: MockerFixture, tmp_path: Path) -> None: + """Tests that _reset_must_change_variables empties a populated file and preserves its other keys.""" + mocker.patch("RUFAS.e2e_test_results_handler.OutputManager.__init__", return_value=None) + add_warning = mocker.patch("RUFAS.e2e_test_results_handler.OutputManager.add_warning") + file_path = tmp_path / "must_change.json" + write_must_change_file(file_path, {"description": "keep me", MUST_CHANGE_VARIABLES_KEY: ["A.x", "B.z"]}) + path_sets = [make_result_path_set(str(file_path)), make_result_path_set(str(file_path))] + + E2ETestResultsHandler._reset_must_change_variables(path_sets) + + with open(file_path, "r", encoding="utf-8") as file: + contents = json.load(file) + assert contents == {"description": "keep me", MUST_CHANGE_VARIABLES_KEY: []} + add_warning.assert_called_once() + + +@pytest.mark.parametrize( + "file_contents", + [ + {"description": "keep me", MUST_CHANGE_VARIABLES_KEY: []}, + "{not valid json", + ], +) +def test_reset_must_change_variables_leaves_file_untouched( + mocker: MockerFixture, tmp_path: Path, file_contents: Any +) -> None: + """Tests that _reset_must_change_variables does not rewrite empty or unparsable files.""" + mocker.patch("RUFAS.e2e_test_results_handler.OutputManager.__init__", return_value=None) + mocker.patch("RUFAS.e2e_test_results_handler.OutputManager.add_warning") + file_path = tmp_path / "must_change.json" + write_must_change_file(file_path, file_contents) + original_contents = file_path.read_text(encoding="utf-8") + + E2ETestResultsHandler._reset_must_change_variables([make_result_path_set(str(file_path))]) + + assert file_path.read_text(encoding="utf-8") == original_contents + + +def test_reset_must_change_variables_missing_file(mocker: MockerFixture, tmp_path: Path) -> None: + """Tests that _reset_must_change_variables ignores missing files.""" + mocker.patch("RUFAS.e2e_test_results_handler.OutputManager.__init__", return_value=None) + add_warning = mocker.patch("RUFAS.e2e_test_results_handler.OutputManager.add_warning") + + E2ETestResultsHandler._reset_must_change_variables([make_result_path_set(str(tmp_path / "no_such_file.json"))]) + + add_warning.assert_not_called() + + +@pytest.mark.parametrize( + "actual_results, must_change_names, expect_passing, expect_error_count, expect_changed, expect_satisfied," + " expect_violations", + [ + # Must-change variable changed, everything else matches: the run passes. + ({"A.x": {"values": [5.0]}, "A.y": {"values": [2.0]}}, ["A.x"], True, 0, None, ["A.x"], set()), + # Must-change variable did not change: the run fails. + ({"A.x": {"values": [1.0]}, "A.y": {"values": [2.0]}}, ["A.x"], False, 1, None, [], {"A.x"}), + # Must-change variable missing from the actual results: the run fails. + ({"A.y": {"values": [2.0]}}, ["A.x"], False, 1, None, [], {"A.x"}), + # Unflagged variable changed: the run fails and the variable is compiled into changed_variables. + ({"A.x": {"values": [1.0]}, "A.y": {"values": [9.0]}}, [], False, 1, ["A.y"], None, None), + # Flagged variable does not exist in the expected results: configuration error. + ({"A.x": {"values": [1.0]}, "A.y": {"values": [2.0]}}, ["A.z"], True, 1, None, None, None), + ], +) +def test_compare_actual_and_expected_results_with_must_change( + mocker: MockerFixture, + tmp_path: Path, + actual_results: dict[str, Any], + must_change_names: list[str], + expect_passing: bool, + expect_error_count: int, + expect_changed: list[str] | None, + expect_satisfied: list[str] | None, + expect_violations: set[str] | None, +) -> None: + """End-to-end tests of compare_actual_and_expected_test_results with must-change variables, on real files.""" + expected_results = {"A.x": {"values": [1.0]}, "A.y": {"values": [2.0]}} + json_output_path = tmp_path / "output" + json_output_path.mkdir() + with open(json_output_path / "actual_prefix_results.json", "w", encoding="utf-8") as file: + json.dump(actual_results, file) + expected_results_path = tmp_path / "e2e_json_test_filter.json" + with open(expected_results_path, "w", encoding="utf-8") as file: + json.dump({"name": "test", "filters": ["A.*"], "expected_results": expected_results}, file) + must_change_path = tmp_path / "must_change_variables.json" + write_must_change_file(must_change_path, {MUST_CHANGE_VARIABLES_KEY: must_change_names}) + path_set = ResultPathType("Animal", str(expected_results_path), "actual_prefix_", 0.1, str(must_change_path)) + mocker.patch.object(E2ETestResultsHandler, "_get_test_result_paths", return_value=[path_set]) + mocker.patch("RUFAS.e2e_test_results_handler.OutputManager.__init__", return_value=None) + mocker.patch("RUFAS.e2e_test_results_handler.OutputManager.add_log") + add_error = mocker.patch("RUFAS.e2e_test_results_handler.OutputManager.add_error") + add_variable = mocker.patch("RUFAS.e2e_test_results_handler.OutputManager.add_variable") + + E2ETestResultsHandler.compare_actual_and_expected_test_results(json_output_path, None, "dummy_prefix") + + reported = {call.args[0]: call.args[1] for call in add_variable.call_args_list} + assert reported["end_to_end_testing_passing"] is expect_passing + assert add_error.call_count == expect_error_count + if expect_changed is None: + assert "changed_variables" not in reported + else: + assert reported["changed_variables"] == expect_changed + if expect_satisfied is None: + assert "must_change_satisfied" not in reported + else: + assert reported["must_change_satisfied"] == expect_satisfied + if expect_violations is None: + assert "must_change_violations" not in reported + else: + assert set(reported["must_change_violations"].keys()) == expect_violations From 8d885c9232461b658b7c1319ef44ed1e8150a6f2 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Fri, 31 Jul 2026 07:25:31 +0000 Subject: [PATCH 2/3] Apply Black Formatting From ab08b85fa7a38def24304239f4508fa09b08672f Mon Sep 17 00:00:00 2001 From: matthew7838 Date: Fri, 31 Jul 2026 07:30:57 +0000 Subject: [PATCH 3/3] Update badges on README --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index c378c18f0c..bc794b53ec 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ -[![Flake8](https://img.shields.io/badge/Flake8-passed-brightgreen)](https://github.com/RuminantFarmSystems/MASM/actions/workflows/combined_format_lint_test_mypy.yml) +[![Flake8](https://img.shields.io/badge/Flake8-failed-red)](https://github.com/RuminantFarmSystems/MASM/actions/workflows/combined_format_lint_test_mypy.yml) [![Pytest](https://img.shields.io/badge/Pytest-passed-brightgreen)](https://github.com/RuminantFarmSystems/MASM/actions/workflows/combined_format_lint_test_mypy.yml) [![Coverage](https://img.shields.io/badge/Coverage-99%25-brightgreen)](https://github.com/RuminantFarmSystems/MASM/actions/workflows/combined_format_lint_test_mypy.yml) -[![Mypy](https://img.shields.io/badge/Mypy-1135%20errors-red)](https://github.com/RuminantFarmSystems/MASM/actions/workflows/combined_format_lint_test_mypy.yml) +[![Mypy](https://img.shields.io/badge/Mypy-1134%20errors-red)](https://github.com/RuminantFarmSystems/MASM/actions/workflows/combined_format_lint_test_mypy.yml) # RuFaS: Ruminant Farm Systems