[MAGNOLIA-6092] Hamcrest UtilMatchers: a pattern for asserting an exception is thrown Created: 18/Feb/15  Updated: 22/May/15  Resolved: 12/May/15

Status: Closed
Project: Magnolia
Component/s: testing
Affects Version/s: None
Fix Version/s: 5.4

Type: New Feature Priority: Neutral
Reporter: Magnolia International Assignee: Magnolia International
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
relation
is related to MAGNOLIA-3792 A few custom Hamcrest matchers Closed
is related to MAGNOLIA-6050 Hamcrest: more exception matchers Closed
is related to MAGNOLIA-6063 new Hamcrest Matcher to help build ch... Closed
Template:
Acceptance criteria:
Empty
Date of First Response:

 Description   

In our current unit tests, we use a mixture of 2 patterns to assert the certain code throws certain exception:

    @Test(expected = IllegalArgumentException.class)
    public void testGetTitleThrowsException() {
        new MetaData(root).getTitle();
    }
    @Test
    public void testGetTitleThrowsException() {
        try {
          new MetaData(root).getTitle();
          fail("should have failed");
        } catch (Throwable t) {
          // various assertions on t, possibly using info.magnolia.test.hamcrest.UtilMatchers 
        }
    }

The former is concise, but leaves no place for signaling the intent of the test, and provides no way to check the actual exception; it's good enough in some cases, but in others, we really want to make sure we're getting the precise exception we think we're getting.

The latter is much more precise, but is a tad verbose, maybe a bit difficult to read, and can be error prone (it's easy to forget the call to fail(), for example)

I propose the following, implemented using a new matcher:


import static info.magnolia.test.hamcrest.UtilMatchers.*;
import static org.junit.Assert.*;
import info.magnolia.test.hamcrest.Execution;

...

    @Test
    public void throwMatcherSampleUsage() {
        assertThat(new Execution(){
            public void evaluate() throws Exception {
                // do something that fails, such as
                System.out.println(1/0);
            }
        }, throwMatcher(isExceptionWithMatchingMessage(ArithmeticException.class, "/ by zero")));
    }

or, slightly differently:

    @Test
    public void throwMatcherSampleUsage() {
        final Execution shouldFail = new Execution() {
            @Override
            public void evaluate() throws Exception {
                // do something that fails, such as
                System.out.println(1 / 0);
            }
        };
        assertThat(shouldFail, throwMatcher(isExceptionWithMatchingMessage(ArithmeticException.class, "/ by zero")));
    }

Additionally, we can even have the same principle to assert that a piece of code does not throw any exception:

 assertThat(new Execution() {
            @Override
            public void evaluate() throws Exception {
                // yay
            }
        }, noExceptionThrown());

And here's a sample of how it'd look like if we used Java 8's lambdas:

        assertThat(() -> System.out.print(1 / 0), throwsException(isExceptionWithMatchingMessage(ArithmeticException.class, "/ by zero")));


 Comments   
Comment by Magnolia International [ 18/Feb/15 ]

Pushed into branch feature/MAGNOLIA-6092-matcher-for-thrown-exception so this can be reviewed/discussed or merged

Comment by Michael Mühlebach [ 19/Feb/15 ]

Would be really helpful. But I have an idea for a more generic matcher.
What do you think about something that looks a little bit more like:

assertThat(...., thrownIsOfInstance(ArithmeticException.class)
                 .hasMessageContaining("by zero")
                 .hasNoCause());

Of course beside hasMessageContaining(), hasMessage() hasMessageStartingWith(), hasMessageEndingWith() would make sense as well.

Comment by Jan Haderka [ 20/Feb/15 ]

@Michael great idea. Care to update branch to add impl for this?

Comment by Magnolia International [ 25/Feb/15 ]
  • hasMessage*: -1; withMessage() that will take another Matcher<String> as an argument would be better.
  • chaining matchers: +1; I've actually also started work on that with MAGNOLIA-6063 (it looks like that's been merged to master via the config branch?)
  • the topic here is not just how to match the exception but more importantly how we "structure" the test code to catch-and-match
Comment by Magnolia International [ 25/Feb/15 ]

Replaced usage of info.magnolia.test.Assertion by a specialized interface (easier to extract from core); added lambda sample.

Comment by Magnolia International [ 25/Feb/15 ]

I would like to propose to split out further improvement on exception matching to another task; this concerns UtilMatchers.isExceptionWith* methods, which pre-existed this ticket.

This ticket is about UtilMatchers.throwsException() and throwsNothing.

Comment by Magnolia International [ 12/May/15 ]

Reopening - moving these matchers to ExecutionMatcher.

Comment by Magnolia International [ 12/May/15 ]

Re-done on feature/exception-matchers-MAGNOLIA-6050 branch. Please review as a whole with MAGNOLIA-6050.

Comment by Michael Mühlebach [ 21/May/15 ]

Review comments:
Commits

  • Is it planned to squash the two last commits (0751f3a, adfe7e7)? If no, 0751f3a would not compile because ExecutionMatcherTest uses the removed methods isException...
  • Refering to both issues in the commit message looks odd. Excpecially because in the text and code it looks like 6050 is replacing 6092.

ChainingMatcher

  • Line 84: replace " " + "with" + " " with " with "

ExactTypeMatcher

  • has some commented code lines: remove
  • is missing static factory methods (or visability should be reduced, if it is only a utility class for other matchers)

ExceptionMatcher

  • make exact matching explicit in the name (instaneOf and ofType sound like they do the same): why not have #ofType() and #ofExactType() instead of #instanceOf() and #ofType()

ExecutionMatcher

  • Migrate magnolia to java 1.8 and use Lambdas
Comment by Magnolia International [ 21/May/15 ]
  • Ended up renaming ExactTypeMatcher to StrictInstanceOf to be closer to Hamcrest's InstanceOf; renamed factory method in ExceptionMatcher.
  • Kept separate commits for readability
  • Added clarifying comments
Generated at Mon Feb 12 04:11:14 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.