Skip to content

Commit

Permalink
Don't ignore errors in the Jasmine suite start/end stages
Browse files Browse the repository at this point in the history
Currently errors in `afterAll` are logged, but don't fail the tests.
This could cause new errors during test teardown to go by unnoticed.

Moreover, the integration test use a different reporting mechanism which
also handled errors differently (this is extra reason to do mozilla#12730).

This patch fixes the issues by consistently handling errors in
`suiteStarted` and `suiteDone` in both reporting mechanisms.

Fixes mozilla#18319.
  • Loading branch information
timvandermeij committed Jun 23, 2024
1 parent 6784124 commit 2f3bf6f
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 9 deletions.
17 changes: 15 additions & 2 deletions test/integration-boot.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ async function runTests(results) {
jasmineDone(suiteInfo) {},
jasmineStarted(suiteInfo) {},
specDone(result) {
// Report on the result of individual tests.
++results.runs;
if (result.failedExpectations.length > 0) {
++results.failures;
Expand All @@ -53,8 +54,20 @@ async function runTests(results) {
}
},
specStarted(result) {},
suiteDone(result) {},
suiteStarted(result) {},
suiteDone(result) {
// Report on the result of `afterAll` invocations.
if (result.failedExpectations.length > 0) {
++results.failures;
console.log(`TEST-UNEXPECTED-FAIL | ${result.description}`);
}
},
suiteStarted(result) {
// Report on the result of `beforeAll` invocations.
if (result.failedExpectations.length > 0) {
++results.failures;
console.log(`TEST-UNEXPECTED-FAIL | ${result.description}`);
}
},
});

return jasmine.execute();
Expand Down
19 changes: 12 additions & 7 deletions test/unit/testreporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,7 @@ const TestReporter = function (browser) {
};

this.suiteStarted = function (result) {
// Normally suite starts don't have to be reported because the individual
// specs inside them are reported, but it can happen that the suite cannot
// start, for instance due to an uncaught exception in `beforeEach`. This
// is problematic because the specs inside the suite will never be found
// and run, so if we don't report the suite start failure here it would be
// ignored silently, leading to passing tests even though some did not run.
// Report on the result of `beforeAll` invocations.
if (result.failedExpectations.length > 0) {
let failedMessages = "";
for (const item of result.failedExpectations) {
Expand All @@ -76,6 +71,7 @@ const TestReporter = function (browser) {
this.specStarted = function (result) {};

this.specDone = function (result) {
// Report on the result of individual tests.
if (result.failedExpectations.length === 0) {
sendResult("TEST-PASSED", result.description);
} else {
Expand All @@ -87,7 +83,16 @@ const TestReporter = function (browser) {
}
};

this.suiteDone = function (result) {};
this.suiteDone = function (result) {
// Report on the result of `afterAll` invocations.
if (result.failedExpectations.length > 0) {
let failedMessages = "";
for (const item of result.failedExpectations) {
failedMessages += `${item.message} `;
}
sendResult("TEST-UNEXPECTED-FAIL", result.description, failedMessages);
}
};

this.jasmineDone = function () {
// Give the test runner some time process any queued requests.
Expand Down

0 comments on commit 2f3bf6f

Please sign in to comment.