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

Re-structured assertion code to be based on predicates. #328

Merged
merged 1 commit into from Dec 27, 2022

Conversation

topbadger
Copy link

@topbadger topbadger commented Dec 26, 2022

By re-structuring the assertion logic to be based upon predicates, the following enhancements have been made:

Custom Predicates

Consumers can now provide custom predicates so that they can assert on LoggingEvents in specific ways in addition to the ways previously supported by the library. This also provides a solution to #188.

Example:

assertThat(logger)
    .hasLogged(event -> event.getFormattedMessage().startsWith("First section of long log message"));

MDC Comparison Strategy

The MDC comparison strategy can now be set by consumers.

  • The full MDC context isn't always relevant to the test, yet the assertions previously enforced this check. The result was that people were unable to use the fluent assertions we provide in a number of scenarios.
  • The previous behaviour of requiring the MDC contents to match exactly has been retained as the default.

Example:

assertThat(logger)
    .usingMdcComparator(MdcComparator.IGNORING)
    .hasLogged(warn("Some log message"));

Enhanced Assertion Failure Messages

Assertion failure messages have been enhanced to show the list of LoggingEvents that were actually captured to make debugging the cause of the failure easier.

Example:

Failed to find event:
  LoggingEvent{level=ERROR, mdc={}, marker=Optional.empty, throwable=Optional[throwable], message='There was a problem!', arguments=[]}

The logger contained the following events:
  - LoggingEvent{level=WARN, mdc={}, marker=Optional.empty, throwable=Optional.empty, message='Some other problem', arguments=[]}
  - LoggingEvent{level=ERROR, mdc={}, marker=Optional.empty, throwable=Optional.empty, message='Yet another problem', arguments=[]}

This enables the following enhancements to be made:
- Consumers can now provide custom predicates so that they can assert on LoggingEvents in whatever way they want to rather than being constrained to the ways exposed by the library.
- MDC comparison strategy can now be set by consumers.
  - The full MDC context isn't always relevant to the test, yet the assertions enforced this check. The result was that people were unable to use the fluent assertions we provide.
  - The previous behaviour of requiring the MDC contents to match exactly has been retained as the default.
- Assertion failure messages have been enhanced to show the list of LoggingEvents that were _actually_ captured to make debugging the cause of the failure easier.
@codeclimate
Copy link

codeclimate bot commented Dec 26, 2022

Code Climate has analyzed commit 3e25a7c and detected 0 issues on this pull request.

View more on Code Climate.


public static class PredicateBuilder {

private static final Predicate<LoggingEvent> IGNORE_PREDICATE = event -> true;
Copy link

Choose a reason for hiding this comment

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

38% of developers fix this issue

UnnecessaryLambda: Returning a lambda from a helper method or saving it in a constant is unnecessary; prefer to implement the functional interface method directly and use a method reference instead.


Suggested change
private static final Predicate<LoggingEvent> IGNORE_PREDICATE = event -> true;
private static boolean ignorePredicate(LoggingEvent event){return true;}

ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


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

}

public PredicateBuilder withArguments(Object... arguments) {
return withArguments(actualArgs -> actualArgs.equals(Arrays.asList(arguments)));
Copy link

Choose a reason for hiding this comment

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

4% of developers fix this issue

UndefinedEquals: Collection does not have well-defined equals behavior.


ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants