Issue 848 replace simple regression tests#903
Conversation
…8-replace-simple-regression-tests
…ices for regression testing
There was a problem hiding this comment.
Pull request overview
This PR replaces the simple diff command used in regression testing with a custom C++ matrix comparison tool that provides more intelligent comparison of XML matrix files with floating-point tolerance.
Changes:
- Implemented a new C++ tool (
compare_matrices.cpp) that parses XML matrix files and compares them with configurable floating-point tolerance - Updated GitHub Actions workflow to compile and use the new comparison tool instead of
diff - Applied formatting improvements to the workflow YAML file
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| Testing/RegressionTesting/compare_matrices.cpp | Complete rewrite of matrix comparison tool with XML parsing, metadata validation, and floating-point tolerance for value comparisons |
| .github/workflows/tests.yml | Added build step for compare_matrices and replaced all diff commands with the new tool; applied formatting fixes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Please see the comments for this review, including the copilot suggestions they reference (apparently, though I wrote these as responses to copilot comments, they don't link to those, so you'll need to scroll to the points in the code to see the copilot suggestions).
|
Do subsequent tests run if an earlier test fails? They used to, but it seems like this action has that fixed. Please confer with @stardriftfx on this. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
stiber
left a comment
stiber
left a comment
There was a problem hiding this comment.
Kyle, please respond to and/or resolve all comments in "Conversation" before requesting my review. Otherwise, I have no way of knowing if any of the matters raised have been addressed. Do the same for all copilot comments; many are at least worth considering.
Subsequent tests should not run if an earlier test fails, after commit 5ece139 which merged the fix from SharedDevelopment to my branch |
2 participants
Closes #848
Description
This PR adds a C++ file that compares two matrices instead of using the previous method of calling diff.
It also changes the tests.yml file to use the new compare_matrices testing method
Checklist (Mandatory for new features)
Testing (Mandatory for all changes)
test-medium-connected.xmlPassedtest-large-long.xmlPassed