Skip to content
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

ObservationContextAssert now asserts ContextView, add parentObservation assertions #3304

Conversation

simonbasle
Copy link
Contributor

@simonbasle simonbasle commented Jul 20, 2022

This PR changes the target type of ObservationContextAssert to ContextView.
The Assert doesn't need the writable aspect of Context, and this switch allows
usage of the Assert on Observation#getContext().

As a result, we can reuse the Assert on actual.getParentObservation().getContext()
when the user passes a Predicate or an asserting Consumer aimed at the parent.

This PR then introduces various assertions around the getParentObservation().

TODO:

  • javadoc
  • tests


public SELF hasParentObservationContextMatching(Predicate<? super Observation.ContextView> parentContextViewPredicate, String description) {
hasParentObservation();
ObservationContextAssert.assertThat(this.actual.getParentObservation().getContext())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NULL_DEREFERENCE: object returned by AbstractAssert.actual.getParentObservation() could be null and is dereferenced at line 387.

Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.

When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

if (realParent == null) {
failWithMessage("Observation should have parent <%s>, has none", expectedParent);
}
if (!realParent.equals(expectedParent)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NULL_DEREFERENCE: object realParent last assigned on line 353 could be null and is dereferenced at line 357.

Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.

When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]


public SELF hasParentObservationContextSatisfying(ThrowingConsumer<? super Observation.ContextView> parentContextViewAssertion) {
hasParentObservation();
assertThat(this.actual.getParentObservation().getContext())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NULL_DEREFERENCE: object returned by AbstractAssert.actual.getParentObservation() could be null and is dereferenced at line 373.

Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.

When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]


public SELF hasParentObservationContextMatching(Predicate<? super Observation.ContextView> parentContextViewPredicate) {
hasParentObservation();
assertThat(this.actual.getParentObservation().getContext())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NULL_DEREFERENCE: object returned by AbstractAssert.actual.getParentObservation() could be null and is dereferenced at line 380.

Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.

When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@simonbasle simonbasle marked this pull request as ready for review July 20, 2022 13:59
@shakuzen shakuzen added this to the 1.10.0-M4 milestone Jul 20, 2022
@simonbasle

This comment was marked as resolved.

public SELF hasParentObservationContextMatching(
Predicate<? super Observation.ContextView> parentContextViewPredicate, String description) {
Observation p = checkedParentObservation();
if (!parentContextViewPredicate.test(p.getContext())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NULL_DEREFERENCE: object p last assigned on line 441 could be null and is dereferenced at line 442.

Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.

When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

ThrowingConsumer<Observation.ContextView> parentContextViewAssertion) {
Observation p = checkedParentObservation();
try {
parentContextViewAssertion.accept(p.getContext());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NULL_DEREFERENCE: object p last assigned on line 406 could be null and is dereferenced at line 408.

Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.

When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

public SELF hasParentObservationContextMatching(
Predicate<? super Observation.ContextView> parentContextViewPredicate) {
Observation p = checkedParentObservation();
if (!parentContextViewPredicate.test(p.getContext())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NULL_DEREFERENCE: object p last assigned on line 426 could be null and is dereferenced at line 427.

Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.

When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@marcingrzejszczak marcingrzejszczak merged commit ba37d40 into micrometer-metrics:main Jul 20, 2022
@simonbasle simonbasle deleted the observationParentAssertions branch July 20, 2022 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants