-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat(test): add file/line to Env #6276
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
base: develop
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR enhances test debugging by adding source location information (file and line number) to test environment error messages using C++20's std::source_location. When test transactions fail, the error output will now include the exact file and line where the test was invoked.
Changes:
- Adds
std::source_locationparameters tosubmit(),sign_and_submit(), andpostconditions()methods to capture call site location - Introduces
WithSourceLocwrapper struct to enable implicit conversion with source location capture for variadic template methods - Updates error messages to include file and line information in the format
(filename:line)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/test/jtx/Env.h | Adds WithSourceLoc wrapper struct and updates method signatures to accept source location parameters |
| src/test/jtx/impl/Env.cpp | Implements source location threading through submission methods and formats location info in error messages |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| struct WithSourceLoc | ||
| { | ||
| std::variant<Json::Value, JTx> value; | ||
| std::source_location loc; | ||
|
|
||
| // Non-explicit constructors allow implicit conversion. | ||
| // The default argument for loc is evaluated at the call site. | ||
| WithSourceLoc( | ||
| Json::Value v, | ||
| std::source_location l = std::source_location::current()) | ||
| : value(std::move(v)), loc(l) | ||
| { | ||
| } | ||
|
|
||
| WithSourceLoc( | ||
| JTx v, | ||
| std::source_location l = std::source_location::current()) | ||
| : value(std::move(v)), loc(l) | ||
| { | ||
| } | ||
| }; |
Copilot
AI
Jan 23, 2026
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.
The WithSourceLoc struct lacks documentation explaining why it exists and how it should be used. While the existing comment explains the mechanism, it would benefit from additional documentation describing the intended usage pattern and when developers should use this wrapper.
| auto const file = | ||
| std::string("(") + loc.file_name() + ":" + to_string(loc.line()) + ")"; |
Copilot
AI
Jan 23, 2026
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.
The file location formatting is duplicated in the variable construction. Consider extracting this into a helper function or formatting it inline where used, as it's only referenced twice in close proximity.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #6276 +/- ##
=========================================
- Coverage 79.4% 79.4% -0.0%
=========================================
Files 839 839
Lines 71619 71619
Branches 8236 8236
=========================================
- Hits 56850 56849 -1
- Misses 14769 14770 +1 🚀 New features to boost your workflow:
|
High Level Overview of Change
This PR uses
std::source_locationto add to testEnverror output the file and line location of the call that triggered the failed transaction.There is no change to the source code.
Context of Change
Improved test logging, easier debugging
Type of Change
API Impact
N/A
Test Plan
I used this commit to help me debug a test failure in another branch.