feat(test): add file/line to Env - #6276
Conversation
There was a problem hiding this comment.
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.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #6276 +/- ##
=======================================
Coverage 79.9% 79.9%
=======================================
Files 840 840
Lines 65483 65483
Branches 7255 7251 -4
=======================================
+ Hits 52311 52319 +8
+ Misses 13172 13164 -8 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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) | ||
| { | ||
| } |
There was a problem hiding this comment.
Both constructors can be implemented more concisely on a single line since the bodies are empty. Consider: WithSourceLoc(Json::Value v, std::source_location l = std::source_location::current()) : value(std::move(v)), loc(l) {}
| 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) | |
| { | |
| } | |
| 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) {} |
| namespace test { | ||
| namespace jtx { | ||
|
|
||
| class Env; // Forward declaration |
There was a problem hiding this comment.
I don't think this is needed
| Json::Value or JTx, allowing implicit conversion without template | ||
| argument deduction issues. | ||
| */ | ||
| struct WithSourceLoc |
There was a problem hiding this comment.
May be worth making it a template class so that you don't need to use std::variant and it's clearer.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Oh I thought I didn't approve it. Sorry for the noise. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This change uses `std::source_location` to output the file and line location of the call that triggered a failed transaction.
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.