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(file-list): refresh resolves before 'file_list_modified' event #1551

Conversation

thetrevdev
Copy link
Contributor

file-list - file_list_modified should be emitted before refresh promise resolves

Runner test execution is scheduled after file-list refresh resolves. The web-server gets the file-list from the last file_list_modified event. This was resulting in a race condition where the web server would grab a stale file list.

Closes #1550

self._emitModified = function (immediate) {
immediate ? emit() : throttledEmit()
}
}())
Copy link
Member

Choose a reason for hiding this comment

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

No need to hide these so much, you can just put the outside the constructor like this

function emit (self) {...}
function throttledEmit(self) {...}

...

self._emitModified = function (immediate) {
  immediate ? emit(self) : throttledEmit(self)
}

@dignifiedquire
Copy link
Member

Thanks I left some inline comments

@GitCop
Copy link

GitCop commented Aug 7, 2015

There were the following issues with your Pull Request

  • Commit: 86ab6e4
    • Commits must be in the following format: %{type}(%{scope}): %{description}

Guidelines are available at http://karma-runner.github.io/0.13/dev/git-commit-msg.html


This message was auto-generated by https://gitcop.com

@thetrevdev thetrevdev force-pushed the fix-file-list-refresh-resolves-before-modified-event branch 2 times, most recently from 0be73c0 to ce08f06 Compare August 8, 2015 00:03
@thetrevdev
Copy link
Contributor Author

Updated.
Travis is failing based on newer eslint detecting tab errors on unmodified file.
Master fails as well. Should I fix in a separate pr?

@dignifiedquire
Copy link
Member

Thanks could you rebase onto the latest master please?

@thetrevdev thetrevdev force-pushed the fix-file-list-refresh-resolves-before-modified-event branch from ce08f06 to fe195d7 Compare August 9, 2015 21:33
@thetrevdev
Copy link
Contributor Author

done

@dignifiedquire
Copy link
Member

Thanks

@dignifiedquire
Copy link
Member

Damn can't merge anymore cause I merged the other one first.

change emitModified to emit 'file_list_modified' immediately
during refresh() to remove throttle and race condition during
subsequent runner executions

Closes karma-runner#1550
@thetrevdev thetrevdev force-pushed the fix-file-list-refresh-resolves-before-modified-event branch from fe195d7 to 65f1eca Compare August 9, 2015 22:11
@thetrevdev
Copy link
Contributor Author

Rebased

dignifiedquire added a commit that referenced this pull request Aug 10, 2015
…es-before-modified-event

fix(file-list): refresh resolves before 'file_list_modified' event
@dignifiedquire dignifiedquire merged commit 0a9f6f7 into karma-runner:master Aug 10, 2015
@dignifiedquire
Copy link
Member

Thanks :octocat:

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.

Runner race condition of stale files on subsequent runs
3 participants