Provide native support for expected exceptions

[ashleyfrieze]

There are a few ways to test for some code ending in an exception.

• Use a framework like AssertJ with assertThatThrownBy

E.g.

it("expects a boom", () -> {
   assertThatThrownBy(() -> { throw new Exception("boom!"); }).isInstanceOf(Exception.class)
                                                             .hasMessageContaining("boom");
}

• Employ the ExpectedException rule from JUnit either with junitMixin or even raw with @Rule

E.g.

@Rule
public ExpectedException expectedException = ExpectedException.none();

{
   it("expects a boom", () -> {
         expectedException .expect(NullPointerException.class);
         throw new NullPointerException();
   });
}

• You could even do aroundEach

describe("These all need an exception to be thrown", () -> {
   aroundEach(block -> {
      try {
         block.run();
      } catch (DesiredException e) {
         return;
      }
     fail("Desired exception not thrown. aaagh!");
   });
});

Each of these is plausible (though the last one sucks a bit), but not quite as neat as @Test(expected=SomeException.class) public void myTest() {} that you get in JUnit. Here are two motivations for some alternative:

• A terse syntax for expecting exceptions that's part of the spec
• Ability to define exception expectations hierarchically - i.e. group together in a context or describe a bunch of things that end in the same exception

There are a couple of ways I could imagine doing this and I'd like feedback on which might be desirable:

• As a BlockConfiguration as part of the with syntax
• Using a hook, a bit like let to provide the expected exception similar to ExpectedException but more native to Spectrum

Block Configuraton Approach

This is hierarchical and can be applied anywhere in the tree

e.g.

describe("Suite", () -> {
   // all children are expected to thrown this exception, though they can specialise it
   // or even override it using the same `with` mechanism
   describe("throwing suite", with(expectedException(NullPointerException.class), () -> {
      it("Should throw", () -> {
          throw new NullPointerException();
      });
   }));

   it("doesn't have to throw", () -> {});

   it("requires its own exception", with(expectedException(RuntimeException.class, "boom!"), () -> {
       throw new RuntimeException("boom!");
   }));
});

Hook approach

Let's use ExpectedException within Spectrum, or a clone of it if we don't want it to be too JUnit. What we end up with will be similar to let but will add an expected exception hook instead of a let hook.

e.g.

describe("Some suite", () -> {
   // start with the assumption that nothing throws, but each spec can modify the expectation
   Supplier<ExpectedException> expectedException = expectException(ExpectedException.none());
   it("wants an exception to be thrown", () -> {
       expectedException .get().expect(NullPointerException.class);
       throw new NullPointerException();
   });
});

// or

describe("Some suite", () -> {
   // Start with the assumption that everything will throw
   Supplier<ExpectedException> expectedException = expectException(NullPointerException.class);
   it("wants an exception to be thrown", () -> {
       throw new NullPointerException();
   });
});

@greghaskins - your thoughts pls.

[ashleyfrieze]

Warning: using @rule in the Spectrum test clas with ExpectedException has some gotchas. It will bleed state between tests in a way that doesn't happen in JUnit, which creates a new test class instance each time. The JUnitMixin approach will resolve that, but maybe we should provide a Spectrum native mechanism and document the whole subject?

[ashleyfrieze]

It turns out that the @Rule compatibility is ZERO!!!

I've made a POC for a Spectrum native approach to this. Undoubtedly it has some rough edges, but a commit illustrating it is here - ashleyfrieze@6896018 - please offer feedback in the comment thread within this issue. Hopefully when #119 is merged, I can produce a PR for this, which is presently branched off #119's codebase as it uses some of it.

[greghaskins]

I think expected exceptions fall under the realm of an assertion library. Reasons:

1. The @Test(expected=SomeException.class) or a Spectrum-ish with(insertSomeHookName() ... doesn't provide enough granularity either for where the exception gets thrown (could be any line in the test) or for checking the properties of that thrown exception.
2. JUnit4's ExpectedException solves some of that, but having to declare it outside the local test scope and set the expectation before the SUT makes tests hard to follow, in my experience. (Just so much nicer when I can see Arrange, Act, Assert clearly.)
3. There's just a lot of messaging to report and scenarios to handle if you want to provide useful failure messages. Right now, Spectrum focuses just on running the tests and leaving all that complexity to an assertion library. Any time we find ourselves having to explain to the user why a test failed, it's probably something that should be covered by an assertion matcher instead.

The good news, however, is that I'm also not a huge fan of the AssertJ way to assert exception scenarios. It's just verbose and not very clear. My preference would be something like:

expect(() -> obj.doSomething(foo))
  .to(raise(SomeException.class));

And I'm experimenting with creating just that, over in https://github.com/spectrumbdd/spectrum-expectations. This issue has caused me to re-consider whether to there should be a Spectrum-provided set of assertions.

An alternative, runner-only solution might just be some syntactic sugar around try...catch. Like:

Optional<Throwable> thrownException = catchException(() -> obj.doSomething(foo));
assertThat(thrownException).isPresent();
assertThat(thrownException.get().getMessage()).isEqualTo("hello");

[ashleyfrieze]

Did you have a look at what I implemented?

It was quite nice in the end.

Your suggestion for an exception assertion looked similar to assertJ to me.

[greghaskins]

Closing this issue per our discussion on Gitter. For those browsing through this issue in the future (hi there! 👋 ), here's the context:

The core Spectrum test runner does not and should not include any assertions (that’s a separate concern entirely). I would consider ExpectedException an assertion, since it would have to throw an AssertionError and explain why a failure occurred. If it’s anywhere, it would be in a separate assertion library.