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 #4761 - Cleanup temporary directory when doing *Schematron* XMLValidation #4762
Conversation
// #4761 - XMLValidator's dtor should clean up the tmpDir | ||
EXPECT_FALSE(openstudio::filesystem::exists(tmpDir)) << "Expected tmpDir to be deleted: " << 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.
Fails before fix, works after
const auto tmpDir = openstudio::filesystem::create_temporary_directory("xmlvalidation"); | ||
if (tmpDir.empty()) { | ||
LOG_AND_THROW(fmt::format("Failed to create a temporary directory for extracting the stylesheets need to transform the Schematron '{}'", | ||
m_tempDir = openstudio::filesystem::create_temporary_directory("xmlvalidation"); |
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.
store the temp dir
XMLValidator::~XMLValidator() { | ||
if (m_tempDir) { | ||
try { | ||
const auto count = openstudio::filesystem::remove_all(m_tempDir.get()); | ||
logAndStore(Debug, fmt::format("Removed temporary directory with {} files", count)); | ||
} catch (const std::exception& e) { | ||
logAndStore(Warn, fmt::format("Error removing temporary directory at {}, Description: {}", toString(m_tempDir.get()), e.what())); | ||
} | ||
} | ||
} |
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.
Delete temp Dir in dtor
~XMLValidator(); | ||
XMLValidator(const XMLValidator& other) = default; | ||
XMLValidator(XMLValidator&& other) noexcept = default; | ||
XMLValidator& operator=(const XMLValidator& other) = default; | ||
XMLValidator& operator=(XMLValidator&& other) noexcept = default; |
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.
Rule of 5
CI Results for c488e51:
|
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.
Haven't reviewed the code, but tried it out and confirmed that it cleaned up the temp dir in an OpenStudio-HPXML run. Thanks @jmarrec!
Pull request overview
Pull 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.