Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failing to load a (esm) module does not fail the test suite #3572

Closed
jehon opened this issue Nov 13, 2020 · 3 comments · Fixed by #3605 or karronoli/redpen#10 · May be fixed by PDSSnyk/juice-shop#74
Closed

Failing to load a (esm) module does not fail the test suite #3572

jehon opened this issue Nov 13, 2020 · 3 comments · Fixed by #3605 or karronoli/redpen#10 · May be fixed by PDSSnyk/juice-shop#74
Labels

Comments

@jehon
Copy link

jehon commented Nov 13, 2020

When loading (esm) modules, if one module has an error in it (example: it could not import another module due to a typo in the import statement), the test should fail.

Karma config:

module.exports = function (config) {
    config.set({
        ...
        files: [
            { pattern: 'tests/*-test.mjs', **type: 'module'** },
            { pattern: '**/*', watched: false, included: false },
        ],
    });
};

Example code:

import { something } from './somewhere.js';
...

If "somewhere.js" does not exists, the test should fail.

Current Behavior

Today, the test pass, skipping the "failingtest.js" as it did not exists, and report the test suite as successful.

09 11 2020 11:52:49.258:INFO [karma-server]: Karma v5.2.3 server started at http://localhost:9876/
09 11 2020 11:52:49.259:INFO [launcher]: Launching browsers ChromiumHeadless with concurrency unlimited
09 11 2020 11:52:49.263:INFO [launcher]: Starting browser ChromiumHeadless
09 11 2020 11:52:49.542:INFO [Chrome Headless 86.0.4240.183 (Linux x86_64)]: Connected on socket pCX5O55W4Zk2P-JAAAAA with id 38747259
09 11 2020 11:52:49.592:WARN [web-server]: 404: /base/tests/something.js
Chrome Headless 86.0.4240.183 (Linux x86_64): Executed 1 of 1 SUCCESS (0.003 secs / 0.001 secs)
TOTAL: 1 SUCCESS

While obviously, the second test did not run and the whole run should be in error

I have made a small project to show the problem:
=> https://github.com/jehon/example-jasmine
npm run test (single pass)
(npm run test-debug to keep it open)

The output of the test is available here: https://travis-ci.com/github/jehon/example-jasmine/builds/200293863

My Environment:

  • Karma: 5.2.3
  • Karma-jasmine: 4.0.1
@jehon
Copy link
Author

jehon commented Nov 13, 2020

I find here an error handler:

https://github.com/karma-runner/karma/blob/master/static/karma.js#L185

  // error during js file loading (most likely syntax error)
  // we are not going to execute at all. `window.onerror` callback.
  this.error = function (messageOrEvent, source, lineno, colno, error) {

That is not called when a module script fail to load.

@jehon
Copy link
Author

jehon commented Nov 13, 2020

After digging around, I find this:

contextWindow.onerror = function () {
   return self.error.apply(self, arguments)
}

(

contextWindow.onerror = function () {
)
which is not triggered when a module fail to load.

One solution would be to change

scriptTags.push(`<script type="${scriptType}" src="${filePath}" ${crossOriginAttribute}></script>`)

into

scriptTags.push(`<script onerror="throw 'Error loading ${filePath}'" type="${scriptType}" src="${filePath}" ${crossOriginAttribute}></script>`)

https://github.com/karma-runner/karma/blob/master/lib/middleware/karma.js#L196

This lead to this error, which is acceptable to me:

Chrome Headless 86.0.4240.198 (Linux x86_64) ERROR
An error was thrown in afterAll
Uncaught Error loading tests/failing-test.mjs
http://localhost:9876/context.html:2463
Chrome Headless 86.0.4240.198 (Linux x86_64): Executed 1 of 1 ERROR (0.005 secs / 0.001 secs)

If you agree with this, I will make a pull request

johnjbarton added a commit to johnjbarton/karma that referenced this issue Dec 15, 2020
Include failing path. This is important for module tags which otherwise
simply give 404 at the server.

Fixes karma-runner#3572
johnjbarton added a commit that referenced this issue Dec 15, 2020
Include failing path. This is important for module tags which otherwise
simply give 404 at the server.

Add e2e test to verify the message.

Fixes #3572
johnjbarton added a commit to johnjbarton/karma that referenced this issue Dec 24, 2020
johnjbarton added a commit to johnjbarton/karma that referenced this issue Dec 24, 2020
karmarunnerbot pushed a commit that referenced this issue Jan 13, 2021
# [6.0.0](v5.2.3...v6.0.0) (2021-01-13)

### Bug Fixes

* **ci:** abandon browserstack tests for Safari and IE ([#3615](#3615)) ([04a811d](04a811d))
* **client:** do not reset karmaNavigating in unload handler ([#3591](#3591)) ([4a8178f](4a8178f)), closes [#3482](#3482)
* **context:** do not error when karma is navigating ([#3565](#3565)) ([05dc288](05dc288)), closes [#3560](#3560)
* **cve:** update ua-parser-js to 0.7.23 to fix CVE-2020-7793 ([#3584](#3584)) ([f819fa8](f819fa8))
* **cve:** update yargs to 16.1.1 to fix cve-2020-7774 in y18n ([#3578](#3578)) ([3fed0bc](3fed0bc)), closes [#3577](#3577)
* **deps:** bump socket-io to v3 ([#3586](#3586)) ([1b9e1de](1b9e1de)), closes [#3569](#3569)
* **middleware:** catch errors when loading a module ([#3605](#3605)) ([fec972f](fec972f)), closes [#3572](#3572)
* **server:** clean up close-server logic ([#3607](#3607)) ([3fca456](3fca456))
* **test:** clear up clearContext ([#3597](#3597)) ([8997b74](8997b74))
* **test:** mark all second connections reconnects ([#3598](#3598)) ([1c9c2de](1c9c2de))

### Features

* **cli:** error out on unexpected options or parameters ([#3589](#3589)) ([603bbc0](603bbc0))
* **client:** update banner with connection, test status, ping times ([#3611](#3611)) ([4bf90f7](4bf90f7))
* **server:** print stack of unhandledrejections ([#3593](#3593)) ([35a5842](35a5842))
* **server:** remove deprecated static methods ([#3595](#3595)) ([1a65bf1](1a65bf1))
* remove support for running dart code in the browser ([#3592](#3592)) ([7a3bd55](7a3bd55))

### BREAKING CHANGES

* **server:** Deprecated `require('karma').server.start()` and `require('karma').Server.start()` variants were removed from the public API. Instead use canonical form:

```
const { Server } = require('karma');
const server = new Server();
server.start();
```
* **cli:** Karma is more strict and will error out if unknown option or argument is passed to CLI.
* Using Karma to run Dart code in the browser is no longer supported. Use your favorite Dart-to-JS compiler instead.

`dart` file type has been removed without a replacement.

`customFileHandlers` DI token has been removed. Use [`middleware`](http://karma-runner.github.io/5.2/config/configuration-file.html#middleware) to achieve similar functionality.

`customScriptTypes` DI token has been removed. It had no effect, so no replacement is provided.
* **deps:** Some projects have socket.io tests that are version sensitive.
@karmarunnerbot
Copy link
Member

🎉 This issue has been resolved in version 6.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

anthony-redFox pushed a commit to anthony-redFox/karma that referenced this issue May 16, 2023
# [6.0.0](karma-runner/karma@v5.2.3...v6.0.0) (2021-01-13)

### Bug Fixes

* **ci:** abandon browserstack tests for Safari and IE ([karma-runner#3615](karma-runner#3615)) ([04a811d](karma-runner@04a811d))
* **client:** do not reset karmaNavigating in unload handler ([karma-runner#3591](karma-runner#3591)) ([4a8178f](karma-runner@4a8178f)), closes [karma-runner#3482](karma-runner#3482)
* **context:** do not error when karma is navigating ([karma-runner#3565](karma-runner#3565)) ([05dc288](karma-runner@05dc288)), closes [karma-runner#3560](karma-runner#3560)
* **cve:** update ua-parser-js to 0.7.23 to fix CVE-2020-7793 ([karma-runner#3584](karma-runner#3584)) ([f819fa8](karma-runner@f819fa8))
* **cve:** update yargs to 16.1.1 to fix cve-2020-7774 in y18n ([karma-runner#3578](karma-runner#3578)) ([3fed0bc](karma-runner@3fed0bc)), closes [karma-runner#3577](karma-runner#3577)
* **deps:** bump socket-io to v3 ([karma-runner#3586](karma-runner#3586)) ([1b9e1de](karma-runner@1b9e1de)), closes [karma-runner#3569](karma-runner#3569)
* **middleware:** catch errors when loading a module ([karma-runner#3605](karma-runner#3605)) ([fec972f](karma-runner@fec972f)), closes [karma-runner#3572](karma-runner#3572)
* **server:** clean up close-server logic ([karma-runner#3607](karma-runner#3607)) ([3fca456](karma-runner@3fca456))
* **test:** clear up clearContext ([karma-runner#3597](karma-runner#3597)) ([8997b74](karma-runner@8997b74))
* **test:** mark all second connections reconnects ([karma-runner#3598](karma-runner#3598)) ([1c9c2de](karma-runner@1c9c2de))

### Features

* **cli:** error out on unexpected options or parameters ([karma-runner#3589](karma-runner#3589)) ([603bbc0](karma-runner@603bbc0))
* **client:** update banner with connection, test status, ping times ([karma-runner#3611](karma-runner#3611)) ([4bf90f7](karma-runner@4bf90f7))
* **server:** print stack of unhandledrejections ([karma-runner#3593](karma-runner#3593)) ([35a5842](karma-runner@35a5842))
* **server:** remove deprecated static methods ([karma-runner#3595](karma-runner#3595)) ([1a65bf1](karma-runner@1a65bf1))
* remove support for running dart code in the browser ([karma-runner#3592](karma-runner#3592)) ([7a3bd55](karma-runner@7a3bd55))

### BREAKING CHANGES

* **server:** Deprecated `require('karma').server.start()` and `require('karma').Server.start()` variants were removed from the public API. Instead use canonical form:

```
const { Server } = require('karma');
const server = new Server();
server.start();
```
* **cli:** Karma is more strict and will error out if unknown option or argument is passed to CLI.
* Using Karma to run Dart code in the browser is no longer supported. Use your favorite Dart-to-JS compiler instead.

`dart` file type has been removed without a replacement.

`customFileHandlers` DI token has been removed. Use [`middleware`](http://karma-runner.github.io/5.2/config/configuration-file.html#middleware) to achieve similar functionality.

`customScriptTypes` DI token has been removed. It had no effect, so no replacement is provided.
* **deps:** Some projects have socket.io tests that are version sensitive.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment