Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix #5076 - /tmp/xmlvalidation directories not cleaned up for factory methods bclXMLValidator & gbxmlValidator #5079

Merged
merged 2 commits into from
Jan 16, 2024
Merged

Fix #5076 - /tmp/xmlvalidation directories not cleaned up for factory methods bclXMLValidator & gbxmlValidator #5079

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
09dbc92
Add unit test for #5076
jmarrec Jan 16, 2024
c103518
Fix #5076 - /tmp/xmlvalidation directories not cleaned up for factory…
jmarrec Jan 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 24 additions & 0 deletions src/utilities/xml/Test/XMLValidator_GTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,30 @@ TEST_F(XMLValidatorFixture, XMLValidator_schematronToXslt) {
EXPECT_TRUE(openstudio::filesystem::is_regular_file(expectedPath));
}

TEST_F(XMLValidatorFixture, XMLValidator_bclXMLValidator_Cleanup) {
openstudio::path tmpDir;
{
auto xmlValidator = XMLValidator::bclXMLValidator(BCLXMLType::MeasureXML, VersionString(3, 1));
tmpDir = xmlValidator.schemaPath().parent_path();
EXPECT_TRUE(openstudio::filesystem::exists(tmpDir));
EXPECT_TRUE(openstudio::filesystem::is_directory(tmpDir));
}
// #5076 - XMLValidator's dtor should clean up the tmpDir
EXPECT_FALSE(openstudio::filesystem::exists(tmpDir)) << "Expected tmpDir to be deleted: " << tmpDir;
}

TEST_F(XMLValidatorFixture, XMLValidator_gbxmlValidator_Cleanup) {
openstudio::path tmpDir;
{
auto xmlValidator = XMLValidator::gbxmlValidator();
tmpDir = xmlValidator.schemaPath().parent_path();
EXPECT_TRUE(openstudio::filesystem::exists(tmpDir));
EXPECT_TRUE(openstudio::filesystem::is_directory(tmpDir));
}
// #5076 - XMLValidator's dtor should clean up the tmpDir
EXPECT_FALSE(openstudio::filesystem::exists(tmpDir)) << "Expected tmpDir to be deleted: " << tmpDir;
}

TEST_P(GbXMLValidatorParametrizedFixture, XMLValidator_GBXMLvalidator_XSD) {
const auto& [filename, n_warnings, n_errors] = GetParam();

Expand Down
8 changes: 6 additions & 2 deletions src/utilities/xml/XMLValidator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Contributor

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.

return validator;
}

XMLValidator XMLValidator::bclXMLValidator(openstudio::BCLXMLType bclXMLType, const VersionString& schemaVersion) {
Expand Down Expand Up @@ -510,7 +512,9 @@ XMLValidator XMLValidator::bclXMLValidator(openstudio::BCLXMLType bclXMLType, co

const bool quiet = true;
::openstudio::embedded_files::extractFile(fmt::format(":/xml/resources/bcl/{}", schemaName), openstudio::toString(tmpDir), quiet);
return XMLValidator(tmpDir / schemaName);
auto validator = XMLValidator(tmpDir / schemaName);
validator.m_tempDir = tmpDir;
return validator;
}

} // namespace openstudio