Skip to content

Conversation

@jmarrec
Copy link
Collaborator

@jmarrec jmarrec commented Aug 21, 2025

Pull request overview

Pull Request Author

  • Code Change
  • All new and existing tests passes

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

…mLogSink

/media/DataExt4/Software/Others/OpenStudio3/src/measure/test/OSMeasure_GTest.cpp:523: Failure
Expected equality of these values:
  0
  sink.logMessages().size()
    Which is: 1
[BOOST_ASSERT] <2> Assertion false failed on line 585 of bool openstudio::measure::OSRunner::validateUserArguments(const std::vector<openstudio::measure::OSArgument>&, const std::map<std::__cxx11::basic_string<char>, openstudio::measure::OSArgument>&) in file /media/DataExt4/Software/Others/OpenStudio3/src/measure/OSRunner.cpp.
cf #5464 (comment)

```
[openstudio.XMLValidator] <1> xsdValidate.parseFileError: XML error 17.1840: Element 'type': [facet 'enumeration'] The value 'Separator' is not an element of the set {'Boolean', 'Double', 'Integer', 'String', 'Choice', 'Path'}.
    at /home/julien/Downloads/temp/5464/RubyModelMeasure/measure.xml:17 while processing "Separator"
[openstudio.XMLValidator] <1> xsdValidate.parseFileError: XML error 17.1840: Element 'type': [facet 'enumeration'] The value 'Separator' is not an element of the set {'Boolean', 'Double', 'Integer', 'String', 'Choice', 'Path'}.
```
@jmarrec jmarrec requested a review from shorowit August 21, 2025 08:18
@jmarrec jmarrec self-assigned this Aug 21, 2025
@jmarrec jmarrec added severity - Normal Bug component - Measures component - Workflow Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. labels Aug 21, 2025
Comment on lines +412 to +415
if (script_argument.type().value() == OSArgumentType::Separator) {
// skip separators
continue;
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No point validating it.

Comment on lines +477 to +478
arg = OSArgument::makeSeparatorArgument("separator");
result.push_back(arg);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

New Test, failing before

Comment on lines +515 to +524
StringStreamLogSink sink;
sink.setLogLevel(Fatal);

// call with a good value
double_arg.setValue(-1.0);
int_arg.setValue(1.0);
EXPECT_TRUE(measure.run(model, runner, argumentMap));

EXPECT_EQ(0, sink.logMessages().size()) << sink.string();
sink.resetStringStream();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fails before

[ RUN      ] MeasureFixture.ModelMeasureWithSeparator
/media/DataExt4/Software/Others/OpenStudio3/src/measure/test/OSMeasure_GTest.cpp:523: Failure
Expected equality of these values:
  0
  sink.logMessages().size()
    Which is: 1
[BOOST_ASSERT] <2> Assertion false failed on line 585 of bool openstudio::measure::OSRunner::validateUserArguments(const std::vector<openstudio::measure::OSArgument>&, const std::map<std::__cxx11::basic_string<char>, openstudio::measure::OSArgument>&) in file /media/DataExt4/Software/Others/OpenStudio3/src/measure/OSRunner.cpp.

<xs:enumeration value="String"/>
<xs:enumeration value="Choice"/>
<xs:enumeration value="Path"/>
<xs:enumeration value="Separator"/>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was missing cf #5464 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yes, I see there was a second error before:

[openstudio.XMLValidator] <1> xsdValidate.parseFileError: XML error 17.1840: Element 'type': [facet 'enumeration'] The value 'Separator' is not an element of the set {'Boolean', 'Double', 'Integer', 'String', 'Choice', 'Path'}.

@ci-commercialbuildings
Copy link
Collaborator

ci-commercialbuildings commented Aug 21, 2025

CI Results for 9adf6d8:

Copy link
Contributor

@shorowit shorowit left a comment

Choose a reason for hiding this comment

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

Tested the Windows build and got no errors, LGTM. Thanks @jmarrec!

@jmarrec jmarrec merged commit 2b0e50e into develop Aug 25, 2025
2 of 6 checks passed
@jmarrec jmarrec deleted the 5464_SeparatorArgument branch August 25, 2025 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Separator argument type causes validation error

4 participants