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

Race condition results in error "Some of your tests did a full page reload!" #3424

Closed
nicojs opened this issue Feb 16, 2020 · 6 comments · Fixed by #3452 or karronoli/redpen#10 · May be fixed by PDSSnyk/juice-shop#74
Closed

Race condition results in error "Some of your tests did a full page reload!" #3424

nicojs opened this issue Feb 16, 2020 · 6 comments · Fixed by #3452 or karronoli/redpen#10 · May be fixed by PDSSnyk/juice-shop#74
Labels

Comments

@nicojs
Copy link
Contributor

nicojs commented Feb 16, 2020

Expected behaviour

When running Karma with singleRun = false and autoWatch = false I would expect to be able to schedule a new run as soon as the 'old' run is completed.

Actual behaviour

A race condition in clearContext can report a false error in the new run.

Environment Details

  • Karma version (output of karma --version): 4.4.1
  • Relevant part of your karma.config.js file
{
  singleRun: false,
  autoWatch: false
}

Steps to reproduce the behaviour

It is difficult to reproduce since it is a race condition. Essentially it can happen in this scenario:

const karam = require('karma');
const server = new karma.Server({ singleRun: false, watch: false });
await server.start();
function runUntilError() {
  karma.runner.run({}, exitCode => {
    if(exitCode === 0) {
       runUntilError();
    } else {
       console.log('Aha! Run errored');
    }
  });
}
runUntilError()

Triage

The problem is in the complete handler. Results are reported to the server before the context is cleared:

karma/client/karma.js

Lines 230 to 242 in 6235e68

this.complete = function (result) {
if (resultsBuffer.length) {
socket.emit('result', resultsBuffer)
resultsBuffer = []
}
if (self.config.clearContext) {
// give the browser some time to breath, there could be a page reload, but because a bunch of
// tests could run in the same event loop, we wouldn't notice.
setTimeout(function () {
clearContext()
}, 0)
}

The clearContext function sets the reloadingContext flag and navigates away.

karma/client/karma.js

Lines 148 to 152 in 6235e68

function clearContext () {
reloadingContext = true
navigateContextTo('about:blank')
}

Now, just before the browser navigates away, the reloadingContext is reset by the next execution:

reloadingContext = !self.config.clearContext

The concequence of this, is that the error is reported here:

self.error('Some of your tests did a full page reload!')

Some of your tests did a full page reload!

However, I didn't reload any page.

Workaround

If you set client.clearContext to false, the context will not be cleared and there is no risk of this race condition.

@nicojs nicojs changed the title Race condition in clearContext can result in error "Some of your tests did a full page reload!" Race condition results in error "Some of your tests did a full page reload!" Feb 16, 2020
@johnjbarton
Copy link
Contributor

Ok if I understand your analysis, our scenarios are

PASS:

  1. execute test
  2. emit result
  3. clearContext on next event loop
  4. clearContext sets reloadingContext =true
  5. reload on next event loop
  6. onbeforeunload checks reloadingContext
  7. success!

FAIL

  1. execute test
  2. emit result
  3. clearContext on next event loop
  4. clearContext sets reloadingContext =true
  5. reload on next event loop
  6. execute test sets reloadingContext = false
  7. onbeforeunload checks reloadingContext
  8. fail

johnjbarton added a commit to johnjbarton/karma that referenced this issue Mar 25, 2020
Add a delay in execute to ensure that reload events and clear context events are completed first.
Fixes karma-runner#3424
johnjbarton added a commit to johnjbarton/karma that referenced this issue Aug 10, 2020
Add a delay in execute to ensure that reload events and clear context events are completed first.
Fixes karma-runner#3424
johnjbarton added a commit that referenced this issue Aug 10, 2020
Add a delay in execute to ensure that reload events and clear context events are completed first.
Fixes #3424
karmarunnerbot pushed a commit that referenced this issue 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 issue has been resolved in version 5.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@KurtPreston
Copy link

Just reporting that I'm still encountering this issue on Karma 5.2.2

However, setting clearContext to false resolves the issue for me.

@martinraie
Copy link

We started experiencing Some of your tests did a full page reload! error message after upgrading karma 5.1.1 => 5.2.0. First test run is OK but it starts failing since the second run. By default we have clearContext set to false. The problem goes away when setting clearContext to true. Could it be related to this issue?

@aronrodrigues
Copy link

aronrodrigues commented Dec 7, 2020

+1 Karma 5.2.3

@johnjbarton
Copy link
Contributor

Please open a new bug with steps to reproduce. This issue is related to a race in the code and steps to reproduce are essential.

anthony-redFox pushed a commit to anthony-redFox/karma that referenced this issue May 16, 2023
…r#3452)

Add a delay in execute to ensure that reload events and clear context events are completed first.
Fixes karma-runner#3424
anthony-redFox pushed a commit to anthony-redFox/karma that referenced this issue 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