Merge pull request #5044 from NREL/add_result_measure_info

Add missing `add_result_measure_info` so that out.osw step result have the same info as before

resources/workflow/output_test/in.osw

@@ -0,0 +1,9 @@
+{
+	"measure_paths": ["../measures/"],
+	"steps": [
+		{
+			"measure_dir_name": "FakeReport",
+			"arguments": {}
+		}
+	]
+}

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 $<TARGET_FILE:openstudio> "${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

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",
+    }

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)

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")));

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 <boost/filesystem/operations.hpp>
 #include <utilities/idd/IddEnums.hxx>
@@ -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

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);