Errors in afterAll functions are not reported.

[johnjbarton]

Feature request

Current behavior
Errors in afterAll() still allow tests to pass.
Wanted behavior
Tests fail on errors
What is the motivation / use case for changing the behavior?
Errors are bad ;-)

The suiteDone() can take a result argument:
https://github.com/bcaudan/jasmine-spec-reporter/blob/master/src/display/execution-display.ts#L101
jasmine will report afterAll errors as a failedExpectation in that arg.

We need that to 1) report the error 2) ensure the final status is FAILED.

[johnjbarton]

For test

describe('afterall errors', () => {
	afterAll(() => {
		throw new Error('error in afterAll');
	});
	it('should not pass', () => {
		expect(true).toBe(true);
	});
});

Here is the default reporter from jasmine:

./node_modules/jasmine/bin/jasmine.js afterall_throws_spec.js 
Started
.


1 spec, 1 failure
Finished in 0.01 seconds

An error was thrown in an afterAll
AfterAll Error: error in afterAll

Here is the jasmine-spec-reporter result:

[afterall]$ ./node_modules/jasmine/bin/jasmine.js afterall_throws_spec.js 
Jasmine started

  afterall errors
    ✓ should not pass

Executed 1 of 1 spec SUCCESS in 0.011 sec.
[afterall]$ 

I think this is a change in jasmine 2.5.

[bcaudan]

nice catch

[bcaudan]

Available in [email protected]

[gvdp]

Hey guys, sorry to dig up an old issue, but I'm experiencing something similar.
I have an afterAll() block which throws an error, and I'm using version 4.2.1 so it is indeed being 'reported' but my node process to run the tests is still exiting with code 0 : Process finished with exit code 0.
This makes my CI pipeline thinks all tests have passed although there was an error.

Is that an issue in the spec-reporter? Or do I have to address this somewhere else?

[johnjbarton]

I don't think the spec reporter should be exiting the process, generally the testrunner should do that.

…

On Tue, May 15, 2018 at 5:24 AM, gvdp ***@***.***> wrote: Hey guys, sorry to dig up an old issue, but I'm experiencing something similar. I have an afterAll() block which throws an error, and I'm using version 4.2.1 so it is indeed being 'reported' but my node process to run the tests is still exiting with code 0 : Process finished with exit code 0. This makes my CI pipeline thinks all tests have passed although there was an error. Is that an issue in the spec-reporter? Or do I have to address this somewhere else? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#210 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAMjAttyPUUdQ9ruhodK7fvo57B15jgOks5tyskWgaJpZM4Olq-_> .

[bcaudan]

@johnjbarton is right, the reporter is not responsible for determining the result or exiting the process.

I think this issue is probably solved with [email protected].
Try to upgrade your jasmine version if you are on an older one, otherwise your issue should probably be reported at https://github.com/jasmine/jasmine.

[gvdp]

Yeah I agree, I was wondering if maybe the reporter "swallowed" the error somehow but afterwards I tried disabling the reporter and it produced the same behaviour. I was just wondering what you guys would do in such a situation.
It's a protractor test and I found in the style guide that it's bad practice to do assertions/verifications in the afterAll() hook, so it seems to be by design that this wouldn't exit the process. For now I moved my assertions to the afterEach() hook.

[johnjbarton]

Before jasmine 3.0, afterAll() errors were not always reported well and test runners may not handle these 'global' errors very well.