-
Notifications
You must be signed in to change notification settings - Fork 193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix #5076 - /tmp/xmlvalidation directories not cleaned up for factory methods bclXMLValidator & gbxmlValidator #5079
Fix #5076 - /tmp/xmlvalidation directories not cleaned up for factory methods bclXMLValidator & gbxmlValidator #5079
Conversation
… methods bclXMLValidator & gbxmlValidator
3d93ea5
to
c103518
Compare
Ok tests fail before fix, work with fix Before: [==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from XMLValidatorFixture
[ RUN ] XMLValidatorFixture.XMLValidator_bclXMLValidator_Cleanup
/Users/julien/Software/Others/OpenStudio2/src/utilities/xml/Test/XMLValidator_GTest.cpp:171: Failure
Value of: openstudio::filesystem::exists(tmpDir)
Actual: true
Expected: false
Expected tmpDir to be deleted: "/var/folders/bz/sgrc10ls2d9fz7418xw3wgym0000gn/T/xmlvalidation-fac0-5db6-68ba-39e5-1705421114-0"
[ FAILED ] XMLValidatorFixture.XMLValidator_bclXMLValidator_Cleanup (0 ms)
[----------] 1 test from XMLValidatorFixture (0 ms total)
[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (1 ms total)
[ PASSED ] 0 tests.
[ FAILED ] 1 test, listed below:
[ FAILED ] XMLValidatorFixture.XMLValidator_bclXMLValidator_Cleanup
1 FAILED TEST
Start 185: XMLValidatorFixture.XMLValidator_gbxmlValidator_Cleanup
2/2 Test #185: XMLValidatorFixture.XMLValidator_gbxmlValidator_Cleanup ....***Failed 0.01 sec
Running main() from /Users/julien/.conan/data/gtest/1.11.0/_/_/build/dbd8bc9563b5a1f4052cc1968e26d22031450443/source_subfolder/googletest/src/gtest_main.cc
Note: Google Test filter = XMLValidatorFixture.XMLValidator_gbxmlValidator_Cleanup
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from XMLValidatorFixture
[ RUN ] XMLValidatorFixture.XMLValidator_gbxmlValidator_Cleanup
/Users/julien/Software/Others/OpenStudio2/src/utilities/xml/Test/XMLValidator_GTest.cpp:183: Failure
Value of: openstudio::filesystem::exists(tmpDir)
Actual: true
Expected: false
Expected tmpDir to be deleted: "/var/folders/bz/sgrc10ls2d9fz7418xw3wgym0000gn/T/xmlvalidation-6495-53c1-ddfb-bb6b-1705421114-0"
[ FAILED ] XMLValidatorFixture.XMLValidator_gbxmlValidator_Cleanup (1 ms)
[----------] 1 test from XMLValidatorFixture (1 ms total)
[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (1 ms total)
[ PASSED ] 0 tests.
[ FAILED ] 1 test, listed below:
[ FAILED ] XMLValidatorFixture.XMLValidator_gbxmlValidator_Cleanup
1 FAILED TEST
0% tests passed, 2 tests failed out of 2
Total Test time (real) = 0.14 sec
The following tests FAILED:
184 - XMLValidatorFixture.XMLValidator_bclXMLValidator_Cleanup (Failed)
185 - XMLValidatorFixture.XMLValidator_gbxmlValidator_Cleanup (Failed) |
CI Results for c103518:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It fixes the issue so I support this, but maybe we could rethink the interface a little bit.
@@ -478,7 +478,9 @@ XMLValidator XMLValidator::gbxmlValidator() { | |||
} | |||
const bool quiet = true; | |||
::openstudio::embedded_files::extractFile(":/xml/resources/GreenBuildingXML_Ver7.03.xsd", openstudio::toString(tmpDir), quiet); | |||
return XMLValidator(tmpDir / "GreenBuildingXML_Ver7.03.xsd"); | |||
auto validator = XMLValidator(tmpDir / "GreenBuildingXML_Ver7.03.xsd"); | |||
validator.m_tempDir = tmpDir; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels awkward and potentially error prone to need to make this assignment. Maybe we could use a temp directory class like Spawn uses https://github.com/NREL/Spawn/blob/develop/util/temp_directory.hpp and perhaps also move the construction of the temp directory into the construction of the XMLValidator. I understand there might be cases where you don't want XMLValidator to use a temp directory, but perhaps if that is a requirement we could design on overload or something.
Pull request overview
/tmp/xmlvalidation
directories not cleaned up for factory methods bclXMLValidator & gbxmlValidatorPull Request Author
src/model/test
)src/energyplus/Test
)src/osversion/VersionTranslator.cpp
)Labels:
IDDChange
APIChange
Pull Request - Ready for CI
so that CI builds your PRReview Checklist
This will not be exhaustively relevant to every PR.