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

Lock and flush stdout in Window#alert. #6917

Merged
merged 1 commit into from Aug 3, 2015
Merged

Conversation

@Ms2ger
Copy link
Contributor

Ms2ger commented Aug 3, 2015

We use alert() to communicate test results to wptrunner. Unfortunately,
sometimes the alert output is interleaved with other output on stdout,
causing wptrunner to classify the test result as a timeout. I hope this will
avoid that scenario.

Review on Reviewable

We use alert() to communicate test results to wptrunner. Unfortunately,
sometimes the alert output is interleaved with other output on stdout,
causing wptrunner to classify the test result as a timeout. I hope this will
avoid that scenario.
@metajack
Copy link
Contributor

metajack commented Aug 3, 2015

@bors-servo r+

-S-awaiting-review +S-awaiting-merge


Reviewed 1 of 1 files at r1.
Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from the review on Reviewable.io

@bors-servo
Copy link
Contributor

bors-servo commented Aug 3, 2015

📌 Commit 2a7f262 has been approved by metajack

@bors-servo
Copy link
Contributor

bors-servo commented Aug 3, 2015

Testing commit 2a7f262 with merge 440cbf9...

bors-servo pushed a commit that referenced this pull request Aug 3, 2015
Lock and flush stdout in Window#alert.

We use alert() to communicate test results to wptrunner. Unfortunately,
sometimes the alert output is interleaved with other output on stdout,
causing wptrunner to classify the test result as a timeout. I hope this will
avoid that scenario.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6917)
<!-- Reviewable:end -->
@metajack metajack self-assigned this Aug 3, 2015
@bors-servo
Copy link
Contributor

bors-servo commented Aug 3, 2015

💔 Test failed - linux3

@jdm
Copy link
Member

jdm commented Aug 3, 2015

@jdm jdm added S-awaiting-merge and removed S-tests-failed labels Aug 3, 2015
@bors-servo
Copy link
Contributor

bors-servo commented Aug 3, 2015

Testing commit 2a7f262 with merge 8647680...

bors-servo pushed a commit that referenced this pull request Aug 3, 2015
Lock and flush stdout in Window#alert.

We use alert() to communicate test results to wptrunner. Unfortunately,
sometimes the alert output is interleaved with other output on stdout,
causing wptrunner to classify the test result as a timeout. I hope this will
avoid that scenario.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6917)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Aug 3, 2015

☀️ Test successful - android, gonk, linux1, linux2, linux3, mac1, mac2, mac3

@bors-servo bors-servo merged commit 2a7f262 into servo:master Aug 3, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@Ms2ger Ms2ger deleted the Ms2ger:lock-alert branch Aug 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.