Skip to content

Conversation

@jmarrec
Copy link
Collaborator

@jmarrec jmarrec commented Dec 8, 2025

Pull request overview

Pull Request Author

  • Model API Changes / Additions
  • Any new or modified fields have been implemented in the EnergyPlus ForwardTranslator (and ReverseTranslator as appropriate)
  • Model API methods are tested (in src/model/test)
  • EnergyPlus ForwardTranslator Tests (in src/energyplus/Test)
  • If a new object or method, added a test in NREL/OpenStudio-resources: Add Link
  • If needed, added VersionTranslation rules for the objects (src/osversion/VersionTranslator.cpp)
  • Verified that C# bindings built fine on Windows, partial classes used as needed, etc.
  • All new and existing tests passes
  • If methods have been deprecated, update rest of code to use the new methods

Labels:

  • If change to an IDD file, add the label IDDChange
  • If breaking existing API, add the label APIChange
  • If deemed ready, add label Pull Request - Ready for CI so that CI builds your PR

Review Checklist

This will not be exhaustively relevant to every PR.

  • Perform a Code Review on GitHub
  • Code Style, strip trailing whitespace, etc.
  • All related changes have been implemented: model changes, model tests, FT changes, FT tests, VersionTranslation, OS App
  • Labeling is ok
  • If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • If feature, test running new feature, try creative ways to break it
  • CI status: all green or justified

@jmarrec jmarrec self-assigned this Dec 8, 2025
@jmarrec jmarrec added severity - Minor Bug component - Model Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. labels Dec 8, 2025
@github-actions
Copy link

github-actions bot commented Dec 8, 2025

🧪 Test Results Dashboard

Summary

Metric Value
Total Tests 4157
Passed 4146
Failed 11
Errors 0
Skipped 0
Success Rate 99.7%
Generated 2025-12-08 15:01:48 UTC

⚠️ Minor Issues Detected

🔍 Failed Tests (11 failures)

Linux-c++ (11 failures)

BCLFixture.BCLMeasure_CTor_throw_invalid_xml.BCLFixture.BCLMeasure_CTor_throw_invalid_xml (run1)

Error Message:

Failed

Full Details:

No details available
OpenStudioCLI.test_measure_manager.OpenStudioCLI.test_measure_manager (run1)

Error Message:

Failed

Full Details:

No details available
CLITest-test_bundle-bundle.CLITest-test_bundle-bundle (run1)

Error Message:

Failed

Full Details:

No details available
CLITest-test_bundle-bundle_git.CLITest-test_bundle-bundle_git (run1)

Error Message:

Failed

Full Details:

No details available
CLITest-test_bundle-bundle_native_embedded.CLITest-test_bundle-bundle_native_embedded (run1)

Error Message:

Failed

Full Details:

No details available
CLITest-test_bundle-bundle.CLITest-test_bundle-bundle (run3)

Error Message:

Failed

Full Details:

No details available
CLITest-test_bundle-bundle_git.CLITest-test_bundle-bundle_git (run3)

Error Message:

Failed

Full Details:

No details available
CLITest-test_bundle-bundle_native_embedded.CLITest-test_bundle-bundle_native_embedded (run3)

Error Message:

Failed

Full Details:

No details available
CLITest-test_bundle-bundle.CLITest-test_bundle-bundle (run2)

Error Message:

Failed

Full Details:

No details available
CLITest-test_bundle-bundle_git.CLITest-test_bundle-bundle_git (run2)

Error Message:

Failed

Full Details:

No details available
CLITest-test_bundle-bundle_native_embedded.CLITest-test_bundle-bundle_native_embedded (run2)

Error Message:

Failed

Full Details:

No details available

📊 Test Run Information

Run XML File Status
run1 results.xml ✅ Found
run3 results.xml ✅ Found
run2 results.xml ✅ Found

```
[ RUN      ] ModelFixture.Model_nowarn_GenericModelObject_CommentOnly
/Users/julien/Software/Others/OpenStudio/src/model/test/Model_GTest.cpp:1217: Failure
Expected equality of these values:
  0
  sink.logMessages().size()
    Which is: 1
Expected zero log messages but found 1: [openstudio.model.Model] <0> Creating GenericModelObject for IddObjectType 'CommentOnly'.
```
@jmarrec jmarrec force-pushed the 5544_GenericModelObject branch from 97c04c7 to 145f467 Compare December 8, 2025 14:51
Comment on lines +1215 to +1217
sink.setLogLevel(Warn);
EXPECT_TRUE(m.addObject(i));
EXPECT_EQ(0, sink.logMessages().size()) << "Expected zero log messages but found " << sink.logMessages().size() << ": " << sink.string();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Before fix there is 1 Warning

[ RUN      ] ModelFixture.Model_nowarn_GenericModelObject_CommentOnly
/Users/julien/Software/Others/OpenStudio/src/model/test/Model_GTest.cpp:1217: Failure
Expected equality of these values:
  0
  sink.logMessages().size()
    Which is: 1
Expected zero log messages but found 1: [openstudio.model.Model] <0> Creating GenericModelObject for IddObjectType 'CommentOnly'.

Comment on lines +159 to +161
if (object.iddObject().type().value() != IddObjectType::CommentOnly) {
LOG(Warn, "Creating GenericModelObject for IddObjectType '" << object.iddObject().type().valueName() << "'.");
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Protect the Warn

Copy link
Collaborator

@joseph-robertson joseph-robertson left a comment

Choose a reason for hiding this comment

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

LGTM. Only log warning when creating object not of IddObjectType "CommentOnly". And a new test, nice.

@joseph-robertson joseph-robertson merged commit 903d571 into develop Dec 15, 2025
3 of 6 checks passed
@joseph-robertson joseph-robertson deleted the 5544_GenericModelObject branch December 15, 2025 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component - Model Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. severity - Minor Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Should not Warn when loading a model that has comments

3 participants