-
-
Notifications
You must be signed in to change notification settings - Fork 3.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 our terms of service and privacy 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?
Conversation
b6d1042
to
d34e490
Compare
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
to
98c9b6f
Compare
c3d0075
to
abfdfcf
Compare
src/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
void captureUnscopedValue( size_t index, std::string const& value ); | ||
|
||
template<typename T> | ||
void captureUnscopedValues( size_t index, T const& value ) { | ||
captureUnscopedValue( index, Catch::Detail::stringify( value ) ); | ||
} | ||
|
||
template<typename T, typename... Ts> | ||
void captureUnscopedValues( size_t index, T const& value, Ts const&... 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 add bool isScoped
parameter to the constructor of Capturer
, which will be kept to decide whether captureValue
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
#define UNSCOPED_INFO( msg ) (void)(0) | ||
#define WARN( msg ) (void)(0) | ||
#define CAPTURE( ... ) (void)(0) | ||
#define UNSCOPED_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
466e345
to
d1ede1a
Compare
d1ede1a
to
7c551dd
Compare
@horenmar feel free to take a look |
7c551dd
to
9f4c665
Compare
9f4c665
to
5abeb26
Compare
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 access m_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 whether m_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