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

Jump to bottom

Should fail the test cases split to a browser that is disconnected #38 #43

Merged
merged 1 commit into from Feb 17, 2020
Merged

Should fail the test cases split to a browser that is disconnected #38 #43

merged 1 commit into from Feb 17, 2020

Conversation

nvladimirovi
Copy link
Contributor

  • Please check if the PR fulfills these requirements
  • [ + ] The commit message follows our guidelines
  • [ + ] Tests for the changes have been added (for bug fixes / features)
  • [ - ] Docs have been added / updated (for bug fixes / features)
  1. Each time the browser unregister and re-register his shard id become different which causes the strategy to not execute any tests to this browser instance.
  2. For some reason the shard index first time is number and there are no issues in the strategy function but the second time (re-connect) it is string which reflects to no tests for this browser, because his index doesn't "answering" the needed condition. For example: 15 % 3 === '0' -> False, should be True.
  • What is the new behaviour (if this is a feature change)?
  1. Find available shard index (Example: from 0 - 3) and assign it to the new browser instance.
  2. Do casting like: Number(a % b) === Number(c).

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
    No breaking changes, no user changes needed.

  • Other information:

Should fail the test cases split to a browser that is disconnected
353f9b4 
Each time the browser unregister and re-register his shard id become different which causes round-robin to not give tests to this browser instance.

The fix is to find available shard index (in our case from 0 - 3) and assign it to the new browser instance.
For some reason the shard index first time is number and there are no issues in round-robin function but the second time (re-connect) it is string which reflects to no tests for this browser, because his index doesn't "answering" the needed condiditon. For example: 15 % 3 === '0' -> False. The fix is to do Number(a) === Number(b).
@nvladimirovi nvladimirovi mentioned this pull request Aug 14, 2019
Closed
2 tasks
@lanshunfang
Copy link

Testing in my production test suites

@lanshunfang
Copy link

lanshunfang commented Aug 19, 2019 •
edited

Testing report

General result

  • ✅ Partially Passed

Steps 1: Prove the issue is still existing in latest karma-parallel npm packages

npm install karma-parallel

✅ Prove when no Chrome instances are disconnected, the result is expected

  • Disable karma-parallel in karma.conf.json to ensure we don't use karma-parallel
  • Run test cases
  • Observe the result
Chrome 76.0.3809 (Mac OS X 10.14.6): Executed 451 of 467 (skipped 16) SUCCESS (9 mins 25.278 secs / 9 mins 25.08 secs)
TOTAL: 451 SUCCESS
TOTAL: 451 SUCCESS
  • Enable karma-parallel in karma.conf.json to ensure we use karma-parallel
  • Run test cases
  • Observe the result
TOTAL: 451 SUCCESS

HeadlessChrome 76.0.3809 (Mac OS X 10.14.6): Executed 127 of 0 (skipped 5) SUCCESS (5 mins 59.303 secs / 5 mins 20.407 secs)
HeadlessChrome 76.0.3809 (Mac OS X 10.14.6): Executed 99 of 0 (skipped 3) SUCCESS (4 mins 6.281 secs / 3 mins 26.544 secs)
HeadlessChrome 76.0.3809 (Mac OS X 10.14.6): Executed 104 of 0 (skipped 1) SUCCESS (5 mins 12.198 secs / 4 mins 35.47 secs)
HeadlessChrome 76.0.3809 (Mac OS X 10.14.6): Executed 121 of 0 (skipped 7) SUCCESS (5 mins 1.872 secs / 4 mins 21.157 secs)
TOTAL: 451 SUCCESS

🚫Test with disconnected / unresponsive Chrome browser instances with karma-parallel from official npm package

  • Enable karma-parallel in karma.conf.json to ensure we use karma-parallel
  • Run test cases
  • Wait for at least a test case has already run in the Chrome browser instance to interrupt
  • Go to Chrome browser instance to interrupt, Open Chrome Developer Tools, click Sources tab, then click Pause script execution in the bottom left of the Developer tools.
  • Keep doing so to interrupt more other chrome instances, to create more Errors like
19 08 2019 10:27:35.631:WARN [Chrome 76.0.3809 (Mac OS X 10.14.6)]: Disconnected (1 times), because no message in 60000 ms.
Chrome 76.0.3809 (Mac OS X 10.14.6) ERROR
  • As well, open Chrome Developer Tools, click Sources tab, then click Pause on exceptions and check Pause on caught exceptions, to generate errors:
Error: Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL.
  • Observe the testing report
  [karma-parallel] Add single test to prevent failure
    ✓ should prevent on this shard failing by having successful tests (1ms)
    ✓ should prevent on this shard failing by having successful tests (2ms)

TOTAL: 2 FAILED, 225 SUCCESS

Chrome 76.0.3809 (Mac OS X 10.14.6): Executed 1 of 1 SUCCESS (0.027 secs / 0.001 secs)
Chrome 76.0.3809 (Mac OS X 10.14.6): Executed 1 of 1 SUCCESS (0.03 secs / 0.002 secs)
Chrome 76.0.3809 (Mac OS X 10.14.6): Executed 104 of 0 (2 FAILED) (skipped 1) (8 mins 46.355 secs / 8 mins 3.877 secs)
Chrome 76.0.3809 (Mac OS X 10.14.6): Executed 121 of 0 (skipped 7) SUCCESS (5 mins 40.193 secs / 5 mins 0.458 secs)
TOTAL: 2 FAILED, 225 SUCCESS

Observe the result from karma-parallel offcial npm packages

  • Some Chrome instance should show an incorrect total number Y in Executed X of Y SUCCESS (* secs / * secs
  • General TOTAL was reduced to 225 unexpectedly

image

Steps 2: Test with fix from https://github.com/nvladimirovi/karma-parallel/tree/topic/disconnected-browser

Download the PR

  • Download zip from https://github.com/nvladimirovi/karma-parallel/tree/topic/disconnected-browser
  • Unzip it to overwrite node_modules/karma-parallel
  • Enable karma-parallel in karma.conf.json to ensure we use karma-parallel
  • Run test cases
  • Wait for at least a test case has already run in the Chrome browser instance to interrupt
  • Go to Chrome browser instance to interrupt, Open Chrome Developer Tools, click Sources tab, then click Pause script execution in the bottom left of the Developer tools.
  • Keep doing so to interrupt more other chrome instances, to create more Errors like
19 08 2019 10:27:35.631:WARN [Chrome 76.0.3809 (Mac OS X 10.14.6)]: Disconnected (1 times), because no message in 60000 ms.
Chrome 76.0.3809 (Mac OS X 10.14.6) ERROR
  • Observe result as followed
TOTAL: 451 SUCCESS

Chrome 76.0.3809 (Mac OS X 10.14.6): Executed 127 of 132 (skipped 5) SUCCESS (3 mins 27.84 secs / 3 mins 27.797 secs)
Chrome 76.0.3809 (Mac OS X 10.14.6): Executed 99 of 102 (skipped 3) SUCCESS (3 mins 23.798 secs / 3 mins 23.758 secs)
Chrome 76.0.3809 (Mac OS X 10.14.6): Executed 104 of 105 (skipped 1) SUCCESS (2 mins 46.113 secs / 2 mins 46.068 secs)
Chrome 76.0.3809 (Mac OS X 10.14.6): Executed 121 of 128 (skipped 7) SUCCESS (4 mins 38.106 secs / 4 mins 38.035 secs)
TOTAL: 451 SUCCESS

image

  • The result is expected in terms of TOTAL, ERROR, SUCCESS

  • Add done callback to some Jasmine test cases, but don't call it within that case

  • Run the cases and observe the result

TOTAL: 3 FAILED, 532 SUCCESS

Chrome 76.0.3809 (Mac OS X 10.14.6): Executed 127 of 0 (skipped 5) SUCCESS (7 mins 6.502 secs / 6 mins 18.725 secs)
Chrome 76.0.3809 (Mac OS X 10.14.6): Executed 286 of 102 (3 FAILED) (skipped 9) (3 mins 30.969 secs / 10 mins 8.411 secs)
Chrome 76.0.3809 (Mac OS X 10.14.6): Executed 1 of 1 SUCCESS (0.019 secs / 0.002 secs)
Chrome 76.0.3809 (Mac OS X 10.14.6): Executed 121 of 0 (skipped 7) SUCCESS (5 mins 20.232 secs / 4 mins 35.492 secs)
TOTAL: 3 FAILED, 532 SUCCESS

image

  • The result is acceptable to me, but it's not accurate
  • The total number of cases run should be 451, not 532.
  • The round-robin parallel case balancing policy is not followed well. See one of the browsers only run 1 case. It should run more even some test cases it got failed on timeout

@nvladimirovi
Copy link
Contributor Author

nvladimirovi commented Aug 19, 2019 •
edited

Hello @lanshunfang.
Thank you for your feedback, it's very good catch.
I think this is problem with karma itself, because we are not executing the tests (in karma-parallel), also I think there is some specific behaviour related with using of done callback function.

I tested it with debugger keyword which was successful.

The round-robin parallel case balancing policy is not followed well. See one of the browsers only run 1 case. It should run more even some test cases it got failed on timeout
Regarding this issue, if you have one test in this specific browser instance which is getting stucked there is no way to execute more tests on this browser instance, because the tests are executed synchronously.

I was not able to reproduce this issue, what version of karma are you using?

@lanshunfang
Copy link

lanshunfang commented Sep 27, 2019 •
edited

Hey guys, the last release of karma-parallel in npm repo was in 2018. This project seems out of maintenance. How about we start a new project?

@lucasklaassen
Copy link

Fixing this would be HUGE. This project is such a time saver.

@nvladimirovi in your opinion what are the things still needed to do on this?

The test count is inaccurate but is that it?

@nvladimirovi
Copy link
Contributor Author

Hi @lucasklaassen, related with our observations, yes, the test count is the main problem.

@kylewhitaker
Copy link

@joeljeske Could we merge this PR?

@joeljeske
Copy link
Owner

This is a hug fix! I appreciate this time spend to get this right. I agree it isn’t perfect but it’s a step in the right direction.

@joeljeske joeljeske merged commit f85bb08 into joeljeske:master Feb 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Should fail the test cases split to a browser that is disconnected
5 participants
@nvladimirovi @lanshunfang @lucasklaassen @kylewhitaker @joeljeske