foundationdb
foundationdb copied to clipboard
Update coverage tool to parse multi-line TEST macro sequences
The current coverage tool parses the line with a TEST macro for a C++ comment (e.g. // this test does X). This comment must be on the same line as the TEST macro, and with our 120 character line limit this can be restrictive.
We should update the coverage tool to be able to parse the comment over multiple lines. If it makes things easier, we could potentially relocate this comment to be a string argument to the TEST macro itself.
@sfc-gh-ajbeamon can I work on this ?
@sfc-gh-ajbeamon tests fail because of code like this:
There are multiple ways to solve this, not sure which approach is best/preferred. I'd like your input on this matter.
I suppose a reasonable rule for this if we want to keep these as comments is that we could take a comment directly preceding the macro or one on the same line. We probably shouldn't include any trailing comments on subsequent lines since we don't typically use that style.
Of course we could also avoid this by using the approach where the description gets passed to the macro. I think this would be clearer and a more expected syntax as long as we can easily parse it out for the tooling.
I suppose a reasonable rule for this if we want to keep these as comments is that we could take a comment directly preceding the macro or one on the same line. We probably shouldn't include any trailing comments on subsequent lines since we don't typically use that style.
Makes sense, I'll do that. I have on more edge case that might be important;
INJECT_FAULT(function, function, function, tester123, abrakadabra);
// Change feed merge cursor
// Is this a comment?
Here we do not do it on the same line and just start below it because max number of characters. How would you identify if the second comment is important?
Of course we could also avoid this by using the approach where the description gets passed to the macro. I think this would be clearer and a more expected syntax as long as we can easily parse it out for the tooling.
I tried working with it but had a hard time figuring out regex.
Tests failed. Here is a before and after of the output:
------------------------
path
C:\Users\lukas\Desktop\app\foundationdb\fdbclient\NativeAPI.actor.cpp
new
:Tracing Partial TSS Mismatch in stream comparison and storing the rest in FDB:
old
:Tracing Partial TSS Mismatch in stream:
------------------------
path
C:\Users\lukas\Desktop\app\foundationdb\fdbrpc\FlowTransport.actor.cpp
new
:We didn't write everything, so apparently the write buffer is full. Wait for it to be nonfull.:
old
:We didn't write everything, so apparently the write buffer is full. Wait for it to be:
------------------------
path
C:\Users\lukas\Desktop\app\foundationdb\fdbrpc\LoadBalance.actor.h
new
:Tracing Partial TSS Mismatch and storing the rest in FDB:
old
:Tracing Partial TSS Mismatch and storing:
------------------------
path
C:\Users\lukas\Desktop\app\foundationdb\fdbserver\DDTeamCollection.actor.cpp
new
:A removed server is still associated with a team in ShardsAffectedByTeamFailure:
old
:A removed server is still associated with a team in:
------------------------
path
C:\Users\lukas\Desktop\app\foundationdb\fdbserver\DDTeamCollection.actor.cpp
new
:TSS recruitment skipped potential pair because it's in a different dc/datahall:
old
:TSS recruitment skipped potential pair because it's in a:
------------------------
path
C:\Users\lukas\Desktop\app\foundationdb\fdbserver\MoveKeys.actor.cpp
new
:A previous finishMoveKeys for this range committed just as it was cancelled to start this one?:
old
:A previous finishMoveKeys for this range committed just as it was cancelled to:
------------------------
path
C:\Users\lukas\Desktop\app\foundationdb\fdbserver\MoveKeys.actor.cpp
new
:The caller had a transaction in flight that assigned keys to the server. Wait for it to reverse its mistake.:
old
:The caller had a transaction in flight that assigned keys to the server. Wait for it to:
------------------------
path
C:\Users\lukas\Desktop\app\foundationdb\fdbserver\StorageCache.actor.cpp
new
:fetchKeys got future_version or process_behind, so there must be a huge storage lag somewhere. Keep trying.:
old
:fetchKeys got future_version or process_behind, so there must be a huge storage lag:
------------------------
path
C:\Users\lukas\Desktop\app\foundationdb\fdbserver\storageserver.actor.cpp
new
:A fetchKeys completed while we were doing this, so eager might be outdated. Read it again.:
old
:A fetchKeys completed while we were doing this, so eager might be outdated. Read it:
------------------------
path
C:\Users\lukas\Desktop\app\foundationdb\fdbserver\storageserver.actor.cpp
new
:it's possible that the caller had a transaction in flight that assigned keys to the server. Wait for it to reverse its mistake.:
old
:it's possible that the caller had a transaction in flight that assigned keys to the:
------------------------
path
C:\Users\lukas\Desktop\app\foundationdb\contrib\rapidjson\rapidjson\stream.h
new
: See TEST(Reader, CustomStringStream) in readertest.cpp for example.:
old
::
------------------------
Addressed by new CODE_PROBE macros