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

Fix disconnect race condition #54

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nvladimirovi
Copy link
Contributor

@nvladimirovi nvladimirovi commented Mar 19, 2020

Issue:
The previous implementation was not taking under
consideration that two browsers can disconnect at
the same time.

Fix:
Collect all shard indexes in array of disconnected browsers.

Testing Done:

  • Verify the test sets will be reconnected to the correct
    shard index and there will be no 1 of 1 test sets, when
    2 browsers disconnect at the same time.
  • 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)
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    Bug fix.

  • What is the current behavior? (You can also link to an open issue here)
    If two browsers disconnect at the same time the second will create new shard index
    and will cause 1 of 1 test set.

  • What is the new behavior (if this is a feature change)?
    Each disconnected browser shard index will be stored in disconnected browsers array
    and when the new browsers try to reconnect they will get their old shard indexes, ensuring
    no 1 of 1 test sets.

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

  • Other information:

Issue:
The previous implementation was not taking under
consideration that two browsers can disconnect at
the same time.

Fix:
Collect all shard indexes in array of disconnected browsers.

Testing Done:
- Verify the test sets will be reconnected to the correct
shard index and there will be no 1 of 1 test sets, when
2 browsers disconnect at the same time.
@juanmendes
Copy link

juanmendes commented Jun 17, 2020

@joeljeske Can this review be looked at? It fixes #42

@sukhbirsinghsaini
Copy link

Hi @joeljeske

I was also facing this issue in my project, where i have around 10K + test cases and few of them are always skipped (randomly any test case not particular) when the browser is disconnected. I tried consuming this code in my local and it is work fine.

Please can you check and merge this PR.

Thanks

@FrapperColin
Copy link

Hi @juanmendes @sukhbirsinghsaini , did you guys found a workaround to this issue ? We're facing the same one..

@juanmendes
Copy link

Hi @juanmendes @sukhbirsinghsaini , did you guys found a workaround to this issue ? We're facing the same one..

We applied the patch from this pull request

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.

4 participants