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

chore(test): mark all second connections reconnects #3598

Merged
merged 1 commit into from
Dec 23, 2020

Conversation

johnjbarton
Copy link
Contributor

The reconnecting test is flakey, sometimes running the tests twice. This is exactly
the behavior we are trying to prevent. Hard to reproduce.

@AppVeyorBot
Copy link

Build karma 2809 failed (commit c7ae8d177b by @johnjbarton)

@karmarunnerbot
Copy link
Member

Build karma 412 failed (commit c7ae8d177b by @johnjbarton)

@karmarunnerbot
Copy link
Member

Build karma 411 failed (commit c7ae8d177b by @johnjbarton)

@AppVeyorBot
Copy link

Build karma 2810 failed (commit db2134446d by @johnjbarton)

@karmarunnerbot
Copy link
Member

Build karma 413 failed (commit db2134446d by @johnjbarton)

@karmarunnerbot
Copy link
Member

Build karma 412 failed (commit db2134446d by @johnjbarton)

@karmarunnerbot
Copy link
Member

Build karma 414 completed (commit 28ef6c5591 by @johnjbarton)

@AppVeyorBot
Copy link

Build karma 2811 completed (commit 28ef6c5591 by @johnjbarton)

@karmarunnerbot
Copy link
Member

Build karma 413 completed (commit 28ef6c5591 by @johnjbarton)

Copy link
Collaborator

@devoto13 devoto13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Maybe change scope to fix as this affects library code and should probably be included in the changelog.

lib/browser.js Outdated
@@ -41,7 +44,10 @@ class Browser {
}

setState (toState) {
this.log.debug(`${this.state} -> ${toState}`)
this.log.info(`${this.state} -> ${toState}`)
if (`${this.state} -> ${toState}` === 'CONFIGURING -> CONNECTED') {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Is it just a debugging leftover or do we want to keep it? In a later case, maybe add a comment/some message explaining what is it about.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

The reconnecting test is flakey, sometimes running the tests twice. This is exactly
the behavior we are trying to prevent. Hard to reproduce.

Refactoring:
 * rename newBrowser to knownBrowser as appropriate.
 * move browser STATE constant set/check into browser module.
     Browser should be the only place the state is set.
 * pass singleRun and clientConfig into Browser
 * pass isSocketReconnect into browser.reconnect()
 * rename browser.onDisconnect to browser.onSocketDisconnect to distinguish
   the socket from the reload cases.
@AppVeyorBot
Copy link

Build karma 2817 completed (commit 1bef7bc670 by @johnjbarton)

@karmarunnerbot
Copy link
Member

Build karma 420 completed (commit 1bef7bc670 by @johnjbarton)

@karmarunnerbot
Copy link
Member

Build karma 419 completed (commit 1bef7bc670 by @johnjbarton)

@AppVeyorBot
Copy link

Build karma 2818 completed (commit eee829bebe by @johnjbarton)

@karmarunnerbot
Copy link
Member

Build karma 421 completed (commit eee829bebe by @johnjbarton)

@karmarunnerbot
Copy link
Member

Build karma 420 completed (commit eee829bebe by @johnjbarton)

lib/browser.js Show resolved Hide resolved
lib/browser.js Show resolved Hide resolved
lib/server.js Show resolved Hide resolved
@johnjbarton johnjbarton merged commit 1c9c2de into karma-runner:master Dec 23, 2020
karmarunnerbot pushed a commit that referenced this pull request 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 PR is included 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 pull request May 16, 2023
The reconnecting test is flakey, sometimes running the tests twice. This is exactly
the behavior we are trying to prevent. Hard to reproduce.

Refactoring:
 * rename newBrowser to knownBrowser as appropriate.
 * move browser STATE constant set/check into browser module.
     Browser should be the only place the state is set.
 * pass singleRun and clientConfig into Browser
 * pass isSocketReconnect into browser.reconnect()
 * rename browser.onDisconnect to browser.onSocketDisconnect to distinguish
   the socket from the reload cases.
anthony-redFox pushed a commit to anthony-redFox/karma that referenced this pull request 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
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants