-
Notifications
You must be signed in to change notification settings - Fork 849
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
Adjustment of behaviour of hasXAttributesSatisfying and hasXAttribute… #4882
Conversation
…sSatisfyingExactly assertions
Codecov ReportBase: 90.90% // Head: 91.06% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #4882 +/- ##
============================================
+ Coverage 90.90% 91.06% +0.16%
- Complexity 4803 4832 +29
============================================
Files 545 545
Lines 14340 14363 +23
Branches 1383 1368 -15
============================================
+ Hits 13036 13080 +44
+ Misses 897 886 -11
+ Partials 407 397 -10
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
@trask I think this was originally your request. Can you review? Thanks! |
sdk/testing/src/main/java/io/opentelemetry/sdk/testing/assertj/AssertUtil.java
Show resolved
Hide resolved
sdk/testing/src/test/java/io/opentelemetry/sdk/testing/assertj/MetricAssertionsTest.java
Outdated
Show resolved
Hide resolved
sdk/testing/src/test/java/io/opentelemetry/sdk/testing/assertj/AssertUtilTest.java
Outdated
Show resolved
Hide resolved
sdk/logs-testing/src/main/java/io/opentelemetry/sdk/testing/assertj/LogRecordDataAssert.java
Outdated
Show resolved
Hide resolved
sdk/testing/src/main/java/io/opentelemetry/sdk/testing/assertj/EventDataAssert.java
Outdated
Show resolved
Hide resolved
sdk/testing/src/test/java/io/opentelemetry/sdk/testing/assertj/MetricAssertionsTest.java
Outdated
Show resolved
Hide resolved
How should I update this branch with main? Merge main here, or rebase this one on main and force push? |
Merge main branch into this and push please! |
public final PointAssertT hasAttributesSatisfyingExactly( | ||
Iterable<AttributeAssertion> assertions) { | ||
AssertUtil.assertAttributesExactly(actual.getAttributes(), assertions); | ||
return myself; |
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.
Can you add coverage for the new code?
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.
Sure, I'll try to finish this one tomorrow :)
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.
Looks reasonable to me, but want to ensure this meets @trask's requirements.
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'm fine if @trask is ok with this.
Ping @trask 🙂 |
sorry for late reply, I'll look at it this week |
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.
thx @lmonkiewicz!
comments below are just adding the second (very helpful) sentence across all of the javadocs
sdk/logs-testing/src/main/java/io/opentelemetry/sdk/testing/assertj/LogRecordDataAssert.java
Outdated
Show resolved
Hide resolved
sdk/logs-testing/src/main/java/io/opentelemetry/sdk/testing/assertj/LogRecordDataAssert.java
Outdated
Show resolved
Hide resolved
sdk/testing/src/main/java/io/opentelemetry/sdk/testing/assertj/DoubleExemplarAssert.java
Outdated
Show resolved
Hide resolved
sdk/testing/src/main/java/io/opentelemetry/sdk/testing/assertj/DoubleExemplarAssert.java
Outdated
Show resolved
Hide resolved
sdk/testing/src/main/java/io/opentelemetry/sdk/testing/assertj/EventDataAssert.java
Outdated
Show resolved
Hide resolved
sdk/testing/src/main/java/io/opentelemetry/sdk/testing/assertj/EventDataAssert.java
Outdated
Show resolved
Hide resolved
sdk/testing/src/main/java/io/opentelemetry/sdk/testing/assertj/LongExemplarAssert.java
Outdated
Show resolved
Hide resolved
sdk/testing/src/main/java/io/opentelemetry/sdk/testing/assertj/LongExemplarAssert.java
Outdated
Show resolved
Hide resolved
sdk/testing/src/main/java/io/opentelemetry/sdk/testing/assertj/SpanDataAssert.java
Outdated
Show resolved
Hide resolved
sdk/testing/src/main/java/io/opentelemetry/sdk/testing/assertj/SpanDataAssert.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Trask Stalnaker <[email protected]>
open-telemetry#4882) * Adjustment of behaviour of hasXAttributesSatisfying and hasXAttributesSatisfyingExactly assertions * Cleanup * Additional test coverage for new assertion method * Added javadoc on assertions Co-authored-by: Trask Stalnaker <[email protected]> Co-authored-by: Trask Stalnaker <[email protected]>
Adjustment of
hasXAttributesSatisfying
andhasXAttributesSatisfyingExactly
assertions, as described in issueCloses: #4778