-
Notifications
You must be signed in to change notification settings - Fork 828
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
Add helper to SpanDataAssert to assert that span status satisfies a condition #4469
Conversation
Codecov Report
@@ Coverage Diff @@
## main #4469 +/- ##
============================================
- Coverage 90.00% 90.00% -0.01%
- Complexity 5041 5046 +5
============================================
Files 579 580 +1
Lines 15535 15552 +17
Branches 1492 1492
============================================
+ Hits 13983 13998 +15
- Misses 1095 1096 +1
- Partials 457 458 +1
Continue to review full report at Codecov.
|
@@ -455,6 +455,13 @@ public SpanDataAssert hasStatus(StatusData status) { | |||
return this; | |||
} | |||
|
|||
/** Asserts the span has a status satisfying the given condition. */ | |||
public SpanDataAssert hasStatusSatisfying(Consumer<StatusData> condition) { |
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.
Thanks @HaloFour - can you instead add StatusAssert
with things like hasCode
, hasDescription
, and other helpers possibly hasDescriptionMatching(String regex)
if there's anything you'd like to add to it? And this can be Consumer<StatusAssert>
. We've settled on using that pattern and find it is simpler and more fluent then just consumers on the data objects.
/** Returns an assertion for {@link StatusData}. */ | ||
public static StatusDataAssert assertThat(@Nullable StatusData statusData) { | ||
return new StatusDataAssert(statusData); | ||
} |
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.
I don't think we should add a top level assertion for StatusData
. These assertions will always be done in the context of asserting against a particular span.
} | ||
|
||
/** Asserts that the status has a description that satisfies the given condition. */ | ||
public StatusDataAssert hasDescriptionSatisfying(Consumer<StringAssert> condition) { |
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.
Do we need 3 variants for asserting the description contents? Would prefer to keep only the exact match and pattern match until there is more demand.
@HaloFour can you rebase and regenerate the apidiffs? Thanks! |
303bdf3
to
cc1b94e
Compare
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
…y-java into trace-testing-status
Allows test to assert that the status of a span meets a specific criteria instead of having to exactly match a given
StatusData
. This is to help with shared instrumentation tests where the status description may represent the exception that is thrown, but that exception might be different depending on the exact library being instrumented.