Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork3.2k
Add Unscoped Capture Logger#3010
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:devel
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
b6d1042 tod34e490Comparecodecovbot commentedJul 30, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@## devel #3010 +/- ##==========================================+ Coverage 90.92% 90.95% +0.03%========================================== Files 201 201 Lines 8653 8659 +6 ==========================================+ Hits 7867 7875 +8+ Misses 786 784 -2 🚀 New features to boost your workflow:
|
4e18ef2 to98c9b6fCompareUh oh!
There was an error while loading.Please reload this page.
c3d0075 toabfdfcfComparesrc/catch2/catch_message.hpp Outdated
| std::vector<MessageInfo>getMessageDetails()const { | ||
| return m_messages; | ||
| } |
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.
This is not used anymore. For next time, this shouldn't be returning the vector by value, but by reference, and be ref-qualified if needed.
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.
Removed it
src/catch2/catch_message.hpp Outdated
| voidcaptureUnscopedValue(size_t index, std::stringconst& value ); | ||
| template<typename T> | ||
| voidcaptureUnscopedValues(size_t index, Tconst& value ) { | ||
| captureUnscopedValue( index,Catch::Detail::stringify( value ) ); | ||
| } | ||
| template<typename T,typename... Ts> | ||
| voidcaptureUnscopedValues(size_t index, Tconst& value, Tsconst&... values ) { | ||
| captureUnscopedValue( index,Catch::Detail::stringify(value) ); | ||
| captureUnscopedValues( index+1, values... ); | ||
| } | ||
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.
Let's not duplicate these, and instead addbool isScoped parameter to the constructor ofCapturer, which will be kept to decide whethercaptureValue should create scoped or unscoped msg.
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.
yeah modified this
src/catch2/catch_message.cpp Outdated
| getResultCapture().emplaceUnscopedMessage(Catch::MessageBuilder( | ||
| m_messages[index].macroName, | ||
| m_messages[index].lineInfo, | ||
| m_messages[index].type) << m_messages[index].message); |
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.
No need to copy the value into message first and then write it into the msg builder stream.
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.
done
src/catch2/catch_message.hpp Outdated
| #defineUNSCOPED_INFO( msg) (void)(0) | ||
| #defineWARN( msg) (void)(0) | ||
| #defineCAPTURE( ... ) (void)(0) | ||
| #defineUNSCOPED_CAPTURE( ... ) (void)(0) |
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.
Please re-align the empty definitions for the whole block. (And same for the one around line 150)
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.
done
Uh oh!
There was an error while loading.Please reload this page.
466e345 tod1ede1aCompared1ede1a to7c551ddCompareRiyuzak2510 commentedAug 13, 2025
@horenmar feel free to take a look |
7c551dd to9f4c665Compare9f4c665 to5abeb26Compare| std::vector<MessageInfo> m_messages; | ||
| IResultCapture& m_resultCapture; | ||
| size_t m_captured =0; | ||
| bool isScoped; |
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.
Take a second look at how the other class variables are named.
| m_resultCapture.pushScopedMessage( m_messages[index] ); | ||
| m_captured++; | ||
| if(isScoped){ | ||
| assert( index < m_messages.size() ); |
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 assert should live at the start of the function, both branches accessm_messages[index].
| if(index == m_messages.size() -1){ | ||
| m_messages.clear(); | ||
| } |
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.
No need to clear this here, instead check in destructor whetherm_isScoped is true and change it accordingly.
Adding Unscoped Capture Logger as a part of#2954
Description
Added Unscoped Capture Logger Feature as required to print details related to Capture
GitHub Issues
Closes#2954