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

Improve complete browser book keeping & improve request next module conditions #357

Merged
merged 1 commit into from
Sep 11, 2019

Conversation

step2yeung
Copy link
Collaborator

@step2yeung step2yeung commented Sep 10, 2019

This PR fixes two issues:

  1. A browser is considered completed when a after-tests-complete or disconnect is received. Currently, we keep track of the number of completed browser by incrementing a value. This way of keep track of the completed browsers is prone to issues if duplicate events are fired/received.
    To help with this, we keep track of the completed browsers with a map using browserId as the key. This allows unique book keeping of each completed browser.

  2. The load-balance functionality uses moduleDone hook from QUnit to request the next test file to load. Since QUnit allows nested modules, it's possible for moduleDone to be called while there are more modules from the test file to be executed. This can cause inefficient load-balancing since a browser in this situation is not free yet.
    To help with this, only request the next test file if this._qunit.config.queue.length is zero. this._qunit.config.queue is the queue of tests to be executed. If it is not zero, it means there are still test modules to be ran before the browser is considered free.

if (this.finished) {
return;
} else if (commands.get('writeExecutionFile')) {
if (commands.get('writeExecutionFile')) {
Copy link
Collaborator Author

@step2yeung step2yeung Sep 11, 2019

Choose a reason for hiding this comment

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

Removed the check for this.finish. This was needed for should write test-execution json when browser exits case to work.

When a browser exits with timeout, this.finish is already set to true. My understanding of the code, was we wanted to short circuit this function so we don't run browserExitHandler because the way it keeps track of browser completion cannot handle duplicates. But with the new implementation, it is not a problem.

@step2yeung step2yeung merged commit 662719a into ember-cli:master Sep 11, 2019
@step2yeung step2yeung deleted the completebrowsers branch September 11, 2019 22:58
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.

3 participants