Unmask failures hidden by afterEach exceptions #41
Conversation
Signed-off-by: Stu Pollock <spollock@pivotal.io>
stuart-pollock
changed the title
stuart-pollock
changed the title
|
This is intended to address #40 |
Signed-off-by: Philip Kuryloski <pkuryloski@pivotal.io>
|
Good catch on this one, @pivotal-stuart-pollock ! I absolutely agree that the exception in Prior ArtIt's an odd scenario, so I did some quick research into how other tools handle this scenario. I've been trying to keep Spectrum's behavior mostly in line with things that already exist whenever it is possible and makes sense. Normal JUnit 4Standard JUnit runs only the first failing public class FooTest {
@Before
public void before1() {
throw new RuntimeException("boom in before1");
}
@Before
public void before2() {
throw new RuntimeException("boom in before2");
}
@After
public void after1() {
throw new RuntimeException("boom in after 1");
}
@After
public void after2() {
throw new RuntimeException("boom in after 2");
}
@Test
public void doesSomething() throws Exception {
System.out.println("running doesSomething()");
}
}Trace: RSpecRSpec seems to behave similar to JUnit: run the panda = "happy"
RSpec.describe "the panda" do
before do
raise Exception.new("boom before 1")
end
before do
raise Exception.new("boom before 2")
end
after do
raise Exception.new("boom after 1")
end
after do
raise Exception.new("boom after 2")
end
it "should be happy" do
puts "running the spec"
expect(panda).to eq('happy')
end
endResult: JasmineSee my fiddle at https://jsfiddle.net/greghaskins/tq4jxh0p/ . Jasmine throws all errors in both exploding Suggested BehaviorI think RSpec gets it right:
It's also basically the same behavior as JUnit, with the main difference being the explicit order of setup/teardown in a BDD-style runner. So in this example: @RunWith(Spectrum.class)
public class PandaSpec {
{
final String panda = "happy";
describe("the panda", () -> {
beforeEach(() -> {
throw new RuntimeException("boom beforeEach 1");
});
beforeEach(() -> {
throw new RuntimeException("boom beforeEach 2");
});
afterEach(() -> {
throw new RuntimeException("boom afterEach 1");
});
afterEach(() -> {
throw new RuntimeException("boom afterEach 2");
});
it("should be happy", () -> {
System.out.println("running the spec");
assertThat(panda, is("happy"));
});
});
}
}I'd think that Spectrum should report something like What do you think? |
|
Hi @greghaskins, Thanks for providing a very thoughtful and thorough response. What would you think about merging and adding the rspec style behavior on top as an additional enhancement? We came across the issue when performing some Spring Bean injection in a beforeEach (where our configuration had errors), and attempting to reset that configuration in an afterEach, and being really confused by the stacktrace. We imagine this is actually a common scenario, since afterEach's are often used for teardown, and may often fail if their corresponding setup failed. Our initial thought with the pull request was that by preferring to show the beforeEach, a developer could fix that error, run their tests again, and see the afterEach failure. We agree the Rspec behavior seems nicer from a developer perspective, but this change would be a nice incremental step towards a better developer experience. Also, a release with the PR included would allow us to easily share the enhanced behavior with the rest of our team. One additional thing we noticed that surprised us with the examples above is how the "boom afterEach 2" is logged before the "boom afterEach 1" in everything but JUnit. We were actually expecting the BDD frameworks to log in the JUnit order. Interesting, although we agree Spectrum should probably have the BDD behavior instead of JUnit's. |
|
I think that sounds reasonable. The only thing I feel weird about in the interim is just completely swallowing that The ordering of |
exceptions thrown in an afterEach when a beforeEach has already thrown Signed-off-by: Nitya Dhanushkodi <ndhanushkodi@pivotal.io>
|
Excellent, printStackTrace added. Thanks! We didn't realize that the JUnit ordering is undefined, but that makes sense given reflection. |
Signed-off-by: Stu Pollock spollock@pivotal.io