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

feat(server): allow 'exit' listeners to set exit code #3541

Merged

Conversation

matz3
Copy link
Contributor

@matz3 matz3 commented Jul 23, 2020

Currently it seems that it's not possible for reporters to set the exit code asynchronously.
Within the "onRunComplete" event, the results.exitCode must be modified synchronously, otherwise the updated value is not taken into account.

With this change, the reporters (or any other plugin) can pass an exit code to the callback of the 'exit' event.

This is the only existing place I've found to be leveraged without creating a lot of new code and APIs.

Todos

  • Add tests
  • Remove chore(npm): update npmignore to allow install via GH url commit before merge

Context / background

This is currently a problem in the karma-coverage plugin.
See karma-runner/karma-coverage#418 and https://github.com/matz3/issue-karma-coverage-threshold-exit-code

@AppVeyorBot
Copy link

Build karma 2716 completed (commit dcd8f32ae6 by @matz3)

@karmarunnerbot
Copy link
Member

Build karma 317 failed (commit dcd8f32ae6 by @matz3)

@karmarunnerbot
Copy link
Member

Build karma 318 completed (commit a328c65e2a by @matz3)

@karmarunnerbot
Copy link
Member

Build karma 319 failed (commit 27a7cc0b3f by @matz3)

@karmarunnerbot
Copy link
Member

Build karma 318 failed (commit a4c1ebae71 by @matz3)

@AppVeyorBot
Copy link

Build karma 2717 failed (commit 27a7cc0b3f by @matz3)

@matz3 matz3 force-pushed the feat-server-exit-listener-set-exit-code branch from 05c8fc7 to 5a2e2f0 Compare July 29, 2020 05:36
@matz3
Copy link
Contributor Author

matz3 commented Jul 29, 2020

Rebased on top of latest master

@karmarunnerbot
Copy link
Member

Build karma 322 failed (commit bfe6a3e7c5 by @matz3)

@AppVeyorBot
Copy link

Build karma 2720 completed (commit bfe6a3e7c5 by @matz3)

@karmarunnerbot
Copy link
Member

Build karma 321 failed (commit bfe6a3e7c5 by @matz3)

@matz3
Copy link
Contributor Author

matz3 commented Jul 29, 2020

Tests environment on Node 14 seems to be broken. At least I can't see any details about failing tests.

@devoto13
Copy link
Collaborator

Looks like a flake to me, can you try to close and re-open the PR to trigger a new build?

@matz3 matz3 closed this Jul 29, 2020
@matz3 matz3 reopened this Jul 29, 2020
@AppVeyorBot
Copy link

Build karma 2721 failed (commit 14a3cf4429 by @matz3)

@karmarunnerbot
Copy link
Member

Build karma 322 failed (commit 14a3cf4429 by @matz3)

@karmarunnerbot
Copy link
Member

Build karma 323 failed (commit 14a3cf4429 by @matz3)

@matz3
Copy link
Contributor Author

matz3 commented Jul 29, 2020

Done. Still failing.

@devoto13
Copy link
Collaborator

I see, probably something is off with AppVeyor as it fails on npm --version.

@johnjbarton
Copy link
Contributor

retry

@johnjbarton johnjbarton reopened this Aug 10, 2020
@karmarunnerbot
Copy link
Member

Build karma 328 failed (commit be98c48cf4 by @matz3)

@AppVeyorBot
Copy link

Build karma 2726 failed (commit be98c48cf4 by @matz3)

@devoto13
Copy link
Collaborator

@johnjbarton Check out #3544 for the workaround.

@karmarunnerbot
Copy link
Member

Build karma 327 failed (commit be98c48cf4 by @matz3)

@johnjbarton
Copy link
Contributor

and again

@johnjbarton johnjbarton reopened this Aug 10, 2020
@AppVeyorBot
Copy link

Build karma 2728 completed (commit a0b96ad9c1 by @matz3)

@karmarunnerbot
Copy link
Member

Build karma 330 completed (commit a0b96ad9c1 by @matz3)

@karmarunnerbot
Copy link
Member

Build karma 329 completed (commit a0b96ad9c1 by @matz3)

Copy link
Contributor

@johnjbarton johnjbarton left a comment

Choose a reason for hiding this comment

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

While this looks ok I can't read it well in github because the last commit shows diffs wrt to the first commit. Can you please squash these and repush?

Currently it seems that it's not possible for reporters to set the exit
code asynchronously.
Within the "onRunComplete" event, the results.exitCode must be modified
synchronously, otherwise the updated value is not taken into account.

With this change, the reporters (or any other plugin) can pass an exit
code to the callback of the 'exit' event.
@matz3 matz3 force-pushed the feat-server-exit-listener-set-exit-code branch from 5a2e2f0 to b7f7dea Compare August 17, 2020 12:49
@matz3
Copy link
Contributor Author

matz3 commented Aug 17, 2020

I've rebased onto the latest master, squashed the two commits and dropped the chore(npm): update npmignore to allow install via GH url commit.

@AppVeyorBot
Copy link

Build karma 2737 completed (commit e1234ad9c2 by @matz3)

@karmarunnerbot
Copy link
Member

Build karma 339 completed (commit e1234ad9c2 by @matz3)

@karmarunnerbot
Copy link
Member

Build karma 338 completed (commit e1234ad9c2 by @matz3)

@johnjbarton johnjbarton merged commit 7a94d33 into karma-runner:master Aug 17, 2020
@matz3 matz3 deleted the feat-server-exit-listener-set-exit-code branch August 18, 2020 06:45
@matz3
Copy link
Contributor Author

matz3 commented Aug 31, 2020

Any plans for a next release?

karmarunnerbot pushed a commit that referenced this pull request Aug 31, 2020
# [5.2.0](v5.1.1...v5.2.0) (2020-08-31)

### Bug Fixes

* **client:** avoid race between execute and clearContext ([#3452](#3452)) ([8bc5b46](8bc5b46)), closes [#3424](#3424)
* **client:** check in bundled client code into version control ([#3524](#3524)) ([6cd5a3b](6cd5a3b)), closes [/github.com/karma-runner/karma/commit/f5521df7df5cd1201b5dce28dc4e326b1ffc41fd#commitcomment-38967493](https://github.com//github.com/karma-runner/karma/commit/f5521df7df5cd1201b5dce28dc4e326b1ffc41fd/issues/commitcomment-38967493)
* **dependencies:** update dependencies ([#3543](#3543)) ([5db46b7](5db46b7))
* **docs:** Update 03-how-it-works.md ([#3539](#3539)) ([e7cf7b1](e7cf7b1))
* **server:** log error when file loading or preprocessing fails ([#3540](#3540)) ([fc2fd61](fc2fd61))

### Features

* **server:** allow 'exit' listeners to set exit code ([#3541](#3541)) ([7a94d33](7a94d33))
@karmarunnerbot
Copy link
Member

🎉 This PR is included in version 5.2.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
)

Currently it seems that it's not possible for reporters to set the exit
code asynchronously.
Within the "onRunComplete" event, the results.exitCode must be modified
synchronously, otherwise the updated value is not taken into account.

With this change, the reporters (or any other plugin) can pass an exit
code to the callback of the 'exit' event.
anthony-redFox pushed a commit to anthony-redFox/karma that referenced this pull request May 16, 2023
# [5.2.0](karma-runner/karma@v5.1.1...v5.2.0) (2020-08-31)

### Bug Fixes

* **client:** avoid race between execute and clearContext ([karma-runner#3452](karma-runner#3452)) ([8bc5b46](karma-runner@8bc5b46)), closes [karma-runner#3424](karma-runner#3424)
* **client:** check in bundled client code into version control ([karma-runner#3524](karma-runner#3524)) ([6cd5a3b](karma-runner@6cd5a3b)), closes [/github.com/karma-runner/karma/commit/f5521df7df5cd1201b5dce28dc4e326b1ffc41fd#commitcomment-38967493](https://github.com//github.com/karma-runner/karma/commit/f5521df7df5cd1201b5dce28dc4e326b1ffc41fd/issues/commitcomment-38967493)
* **dependencies:** update dependencies ([karma-runner#3543](karma-runner#3543)) ([5db46b7](karma-runner@5db46b7))
* **docs:** Update 03-how-it-works.md ([karma-runner#3539](karma-runner#3539)) ([e7cf7b1](karma-runner@e7cf7b1))
* **server:** log error when file loading or preprocessing fails ([karma-runner#3540](karma-runner#3540)) ([fc2fd61](karma-runner@fc2fd61))

### Features

* **server:** allow 'exit' listeners to set exit code ([karma-runner#3541](karma-runner#3541)) ([7a94d33](karma-runner@7a94d33))
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