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

Allow browserDisconnectTolerance to be set in non-singleRun mode. #2127

Conversation

dtinth
Copy link
Contributor

@dtinth dtinth commented May 10, 2016

We’re encountering a problem where sometimes Karma just disconnects and stops running tests until we restart Karma manually.

We found that some of our JavaScript code intermittently gets stuck when running tests — that Karma considers it a time out. One such example is an infinite loop in the code.

Looking at other issues (#598, #1514) suggests adding browserDisconnectTolerance. I’ve tried that and several other options to no avail.

From further source code inspection, in 19590e1 it turns out that browserDisconnectTolerance is only active in singleRun mode. This pull request makes browserDisconnectTolerance work in non-singleRun mode.

Test case

Here’s a test case to verify that my patch works:

test.js

This test will get stuck in an infinite loop with 75% chance.

describe('example', function () {
  it('ok1', function () { })
  it('ok2', function () { })
  it('bad1', function () {
    if (Math.random() > 0.5) for (;;) ;
  })
  it('bad2', function () {
    if (Math.random() > 0.5) for (;;) ;
  })
  it('ok3', function () { })
})

karma.conf.js

Sets a ridiculously high amount of retries.

module.exports = function (config) {
  config.set({
    basePath: '',
    frameworks: ['mocha'],
    files: [
      'test.js'
    ],
    reporters: [ 'progress' ],
    port: 9988,
    colors: true,
    logLevel: config.LOG_INFO,
    autoWatch: true,
    browsers: [ 'PhantomJS' ],
    browserDisconnectTolerance: 99999999999999999999999999999999999999999999999,
    retryLimit: 999999999999999999999999999999999999999999999999999999999999999,
    plugins: [
      'karma-mocha',
      'karma-phantomjs-launcher',
    ],
    singleRun: false,
  })
}

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@dtinth
Copy link
Contributor Author

dtinth commented May 10, 2016

My employer has signed the CLA for me.

On a related note, new version of eslint-config-standard broke the builds, because this repo gets a minor version bump when a new major version of standard is released. I believe that this problem is out of the scope of this pull request.

@dignifiedquire
Copy link
Member

Thanks, could you please update your commit message to follow our convention: http://karma-runner.github.io/0.13/dev/git-commit-msg.html

@dtinth
Copy link
Contributor Author

dtinth commented May 17, 2016

Thanks for your reply. Will do it tomorrow.

@googlebot
Copy link

CLAs look good, thanks!

@dtinth
Copy link
Contributor Author

dtinth commented May 17, 2016

I’ll put a draft commit message here in case you would like to comment on it before I do the actual amendment tomorrow:

fix(web-server): Restart disconnected browser in non-singleRun mode.

Allow browserDisconnectTolerance config option to be set in non-singleRun mode.

In some cases, the browser would get stuck in an infinite loop (e.g. because of
a faulty code/test). This blocks browser’s event loop, preventing it from
reporting back to Karma. Karma then considers the browser ‘DISCONNECTED’.

Prior to this commit, the `browserDisconnectTolerance` option will only apply
in singleRun mode. In above scenario, the end-user must restart Karma to
continue running tests inside Karma-managed browsers. This commit fixes this
problem by always honoring the aforementioned option.

I marked this as a “fix” because the configuration docs did not mention that this option will not apply in non-singleRun mode. Please let me know if it needs changing.

@dignifiedquire
Copy link
Member

Thanks, commit message looks good to me. Can you please also rebase onto latest master as that should make CI happy again

Allow browserDisconnectTolerance config option to be set in non-singleRun mode.

In some cases, the browser would get stuck in an infinite loop (e.g. because of
a faulty code/test). This blocks browser’s event loop, preventing it from
reporting back to Karma. Karma then considers the browser ‘DISCONNECTED’.

Prior to this commit, the `browserDisconnectTolerance` option will only apply
in singleRun mode. In above scenario, the end-user must restart Karma to
continue running tests inside Karma-managed browsers. This commit fixes this
problem by always honoring the aforementioned option.
@dtinth dtinth force-pushed the feature-allow-browserDisconnectTolerance-to-be-set-in-non-singleRun-mode branch from 9c281ea to f6587dc Compare May 18, 2016 12:38
@dtinth
Copy link
Contributor Author

dtinth commented May 18, 2016

Thanks for the feedback. Rebased. 😉

@dignifiedquire
Copy link
Member

Thanks :octocat:

@dignifiedquire dignifiedquire merged commit 3480c14 into karma-runner:master Jun 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants