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

[wptrunner] Guard against empty command_queue in BrowserManager.send_message #5774

Merged
merged 2 commits into from Oct 22, 2017

Conversation

jeffcarp
Copy link
Contributor

@jeffcarp jeffcarp commented May 3, 2017

This happens when sending the message 'init_failed' and causes wptrunner to hang since self.command_queue doesn't exist at that point.

This patch is something I'm using to keep wptrunner from getting stuck, but there is probably a cleaner solution.


This change is Reviewable

@w3c-bots
Copy link

w3c-bots commented May 3, 2017

Build BROKEN

Started: 2017-08-11 23:43:50
Finished: 2017-08-12 00:02:42

View more information about this build on:

@w3c-bots
Copy link

w3c-bots commented May 3, 2017

View the complete job log.

Firefox (nightly channel)

Testing web-platform-tests at revision b31f26a
Using browser at version BuildID 20170502100421; SourceStamp 48c0fd9c9ec5d68061ea7b59358874ae8da72572
Starting 10 test iterations
No tests run.

@w3c-bots
Copy link

w3c-bots commented May 3, 2017

View the complete job log.

Chrome (unstable channel)

Testing web-platform-tests at revision b31f26a
Using browser at version 60.0.3080.5 dev
Starting 10 test iterations
No tests run.

@jgraham
Copy link
Contributor

jgraham commented May 5, 2017

I think the code here is just broken; self.command_queue is supposed to be set in __init__ but currently isn't. Fixing that seems like the right solution.

jeffcarp and others added 2 commits August 11, 2017 16:38
…message

This happens when sending the message 'init_failed' and causes wptrunne
 to hang. This patch is something I need to keep the tests running, but
there is probably a cleaner solution.
@jeffcarp
Copy link
Contributor Author

@jgraham apologies for the really long latency on this - I just updated the patch with your suggestion.

@jeffcarp
Copy link
Contributor Author

(Tracked in web-platform-tests/results-collection#142)

@w3c-bots
Copy link

w3c-bots commented Oct 11, 2017

Build PASSED

Started: 2017-10-11 20:52:17
Finished: 2017-10-11 21:11:14

View more information about this build on:

@jeffcarp
Copy link
Contributor Author

Looks like the earlier test failure was a fluke, this should be ready to go now.

@jgraham
Copy link
Contributor

jgraham commented Oct 22, 2017

I thought I'd removed the use of command_queue in this class, but I'm not sure now how I did that or whether it got lost in a merge somewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants