From 6989e089d09a69dd009bab15a7f82270424110c3 Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Wed, 15 Nov 2023 23:36:22 +0100 Subject: [PATCH 1/2] Add missing `add_result_measure_info` Found because the OS-resources tests rely on it to locate openstudio_results --- src/workflow/ApplyMeasure.cpp | 4 ++++ src/workflow/Util.cpp | 23 +++++++++++++++++++++++ src/workflow/Util.hpp | 4 ++++ 3 files changed, 31 insertions(+) diff --git a/src/workflow/ApplyMeasure.cpp b/src/workflow/ApplyMeasure.cpp index b60feb89b54..0f8816857f5 100644 --- a/src/workflow/ApplyMeasure.cpp +++ b/src/workflow/ApplyMeasure.cpp @@ -84,6 +84,7 @@ void OSWorkflow::applyMeasures(MeasureType measureType, bool energyplus_output_r LOG(Info, fmt::format("Skipping measure '{}'", measureDirName)); WorkflowStepResult result = runner.result(); runner.incrementStep(); + // addResultMeasureInfo(result, bclMeasure); // TODO: Should I really instantiate the BCLMeasure just for this? result.setStepResult(StepResult::Skip); } @@ -296,8 +297,10 @@ end } catch (const std::exception& e) { runner.registerError(e.what()); if (!energyplus_output_requests) { + WorkflowStepResult result = runner.result(); // incrementStep must be called after run runner.incrementStep(); + workflow::util::addResultMeasureInfo(result, bclMeasure); } ensureBlock(true); throw std::runtime_error(fmt::format("Runner error: Measure '{}' reported an error with [{}]", scriptPath_->generic_string(), e.what())); @@ -309,6 +312,7 @@ end // incrementStep must be called after run runner.incrementStep(); + workflow::util::addResultMeasureInfo(result, bclMeasure); if (auto errors = result.stepErrors(); !errors.empty()) { ensureBlock(true); throw std::runtime_error(fmt::format("Measure '{}' reported an error with [{}]", measureDirName, fmt::join(errors, "\n"))); diff --git a/src/workflow/Util.cpp b/src/workflow/Util.cpp index 04495b9c19c..e2df902b3f1 100644 --- a/src/workflow/Util.cpp +++ b/src/workflow/Util.cpp @@ -18,6 +18,9 @@ #include "../utilities/idf/IdfObject.hpp" #include "../utilities/idd/IddObject.hpp" #include "../utilities/idf/IdfExtensibleGroup.hpp" +#include "../utilities/filetypes/WorkflowStepResult.hpp" +#include "../utilities/bcl/BCLMeasure.hpp" +#include "../utilities/time/DateTime.hpp" #include #include @@ -326,4 +329,24 @@ std::string sanitizeKey(std::string key) { return key; } +bool addResultMeasureInfo(WorkflowStepResult& result, BCLMeasure& measure) { + try { + result.setMeasureType(measure.measureType()); + result.setMeasureName(measure.name()); + result.setMeasureId(measure.uid()); + result.setMeasureVersionId(measure.versionId()); + auto version_modified_ = measure.versionModified(); + if (version_modified_) { + result.setMeasureVersionModified(*version_modified_); + } + result.setMeasureXmlChecksum(measure.xmlChecksum()); + result.setMeasureClassName(measure.className()); + result.setMeasureDisplayName(measure.displayName()); + result.setMeasureTaxonomy(measure.taxonomyTag()); + return true; + } catch (...) { + return false; + } +} + } // namespace openstudio::workflow::util diff --git a/src/workflow/Util.hpp b/src/workflow/Util.hpp index fb8fc6ef617..905fa801746 100644 --- a/src/workflow/Util.hpp +++ b/src/workflow/Util.hpp @@ -15,6 +15,8 @@ namespace model { } class IdfObject; class Workspace; +class WorkflowStepResult; +class BCLMeasure; namespace workflow { @@ -25,6 +27,8 @@ namespace workflow { void gatherReports(const openstudio::filesystem::path& runDirPath, const openstudio::filesystem::path& rootDirPath); + bool addResultMeasureInfo(WorkflowStepResult& result, BCLMeasure& measure); + // Cleans up the run directory (remove epw, .mtr) void cleanup(const openstudio::filesystem::path& runDirPath); From ad85c1a2c99ec81a89655d4bb7800ac2f4972e1b Mon Sep 17 00:00:00 2001 From: Julien Marrec Date: Wed, 15 Nov 2023 23:58:20 +0100 Subject: [PATCH 2/2] Add test for the out.osw --- resources/workflow/output_test/in.osw | 9 +++ src/cli/CMakeLists.txt | 5 ++ src/cli/test/test_output_files.py | 107 ++++++++++++++++++++++++++ src/cli/test/workflow_helpers.py | 1 + 4 files changed, 122 insertions(+) create mode 100644 resources/workflow/output_test/in.osw create mode 100644 src/cli/test/test_output_files.py diff --git a/resources/workflow/output_test/in.osw b/resources/workflow/output_test/in.osw new file mode 100644 index 00000000000..79cae309159 --- /dev/null +++ b/resources/workflow/output_test/in.osw @@ -0,0 +1,9 @@ +{ + "measure_paths": ["../measures/"], + "steps": [ + { + "measure_dir_name": "FakeReport", + "arguments": {} + } + ] +} diff --git a/src/cli/CMakeLists.txt b/src/cli/CMakeLists.txt index a81ac481712..9843551852e 100644 --- a/src/cli/CMakeLists.txt +++ b/src/cli/CMakeLists.txt @@ -265,6 +265,11 @@ if(BUILD_TESTING) WORKING_DIRECTORY "${PROJECT_BINARY_DIR}/resources/workflow/outdated_measures/" ) + add_test(NAME OpenStudioCLI.test_output_files + COMMAND ${Python_EXECUTABLE} -m pytest --verbose --os-cli-path $ "${CMAKE_CURRENT_SOURCE_DIR}/test/test_output_files.py" + WORKING_DIRECTORY "${PROJECT_BINARY_DIR}/resources/workflow/output_test/" + ) + else() # TODO: Remove. Fallback on these for now, as I don't know if CI has pytest installed add_test(NAME OpenStudioCLI.Classic.test_logger_rb diff --git a/src/cli/test/test_output_files.py b/src/cli/test/test_output_files.py new file mode 100644 index 00000000000..45e1297e66e --- /dev/null +++ b/src/cli/test/test_output_files.py @@ -0,0 +1,107 @@ +import json +from pathlib import Path + +import pytest + +from workflow_helpers import run_workflow + + +@pytest.fixture(scope="module", params=[True, False], ids=["labs", "classic"]) +def runWorkflow(osclipath, request): + is_labs = request.param + suffix = "labs" if is_labs else "classic" + runDir, r = run_workflow( + osclipath=osclipath, + base_osw_name="in.osw", + suffix=suffix, + is_labs=is_labs, + verbose=False, + debug=False, + post_process_only=True, + ) + r.check_returncode() + return runDir, is_labs + + +@pytest.mark.parametrize( + "is_labs", + [pytest.param(True, id="labs"), pytest.param(False, id="classic")], +) +def test_(osclipath, is_labs: bool): + suffix = "labs" if is_labs else "classic" + runDir, r = run_workflow( + osclipath=osclipath, + base_osw_name="in.osw", + suffix=suffix, + is_labs=is_labs, + verbose=False, + debug=True, + post_process_only=True, + ) + r.check_returncode() + out_osw_path = Path(f"out_in_{suffix}.osw") + assert out_osw_path.is_file() + out = json.loads(out_osw_path.read_text()) + + assert out["completed_status"] == "Success" + assert out["current_step"] == 1 + + EXPECTED_TOPLEVEL_KEYS = { + "completed_at", + "completed_status", + "current_step", + "file_paths", + "hash", + "measure_paths", + "out_name", + "run_directory", + "started_at", + "steps", + "updated_at", + } + if is_labs: + EXPECTED_TOPLEVEL_KEYS.add("run_options") + assert out.keys() == EXPECTED_TOPLEVEL_KEYS + + assert len(out["steps"]) == 1 + step = out["steps"][0] + step.keys() == {"arguments", "measure_dir_name", "result"} + assert step["arguments"] == {} + assert step["measure_dir_name"] == "FakeReport" + + step_result = step["result"] + assert step_result.keys() == { + "completed_at", + "measure_class_name", + "measure_display_name", + "measure_name", + "measure_taxonomy", + "measure_type", + "measure_uid", + "measure_version_id", + "measure_version_modified", + "measure_xml_checksum", + "started_at", + "stderr", + "stdout", + "step_errors", + "step_files", + "step_final_condition", + "step_info", + "step_result", + "step_values", + "step_warnings", + } + + assert step_result["step_result"] == "Success" + + assert len(step_result["step_info"]) == 1 + assert len(step_result["step_warnings"]) == 1 + assert not step_result["step_errors"] + + len(step_result["step_values"]) == 3 + assert {x["name"] for x in step_result["step_values"]} == { + "fake_report", + "net_site_energy", + "something_with_invalid_chars", + } diff --git a/src/cli/test/workflow_helpers.py b/src/cli/test/workflow_helpers.py index c46419c53a8..d36b1a92e0b 100644 --- a/src/cli/test/workflow_helpers.py +++ b/src/cli/test/workflow_helpers.py @@ -71,6 +71,7 @@ def run_workflow( osw_path = base_osw_path.parent / f"{base_osw_path.stem}_{suffix}.osw" runDir = base_osw_path.parent / f"run_{osw_path.stem}" osw["run_directory"] = str(runDir) + osw["out_name"] = f"out_{osw_path.stem}.osw" if runDir.is_dir(): shutil.rmtree(runDir) runDir.mkdir(exist_ok=False)