Skip to content

/ servo

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

Test namespace prefix for element equality #6554

Merged
merged 1 commit into from Jul 14, 2015
Merged

Test namespace prefix for element equality #6554

merged 1 commit into from Jul 14, 2015
+29 −2

Conversation

@dzbarsky
Copy link
Member

dzbarsky commented Jul 4, 2015

Review on Reviewable
@highfive
Copy link

highfive commented Jul 4, 2015

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @mbrubeck (or someone else) soon.
@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented Jul 4, 2015

Critic review: https://critic.hoppipolla.co.uk/r/5466

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.
@Ms2ger
Copy link
Contributor

Ms2ger commented Jul 5, 2015

If this doesn't fix any test, we should change that.
@dzbarsky
Copy link
Member Author

dzbarsky commented Jul 5, 2015

Yes, I need to write tests for this and the other namespace stuff I've been working on.
@Ms2ger Ms2ger self-assigned this Jul 6, 2015
@dzbarsky
Copy link
Member Author

dzbarsky commented Jul 8, 2015

Doesn't need a test anymore, but I'm not sure how to remove the label.
@jdm jdm removed the C-needs-test label Jul 8, 2015
@dzbarsky dzbarsky force-pushed the dzbarsky:master branch from a9086ff to d497e24 Jul 10, 2015
@Ms2ger
Copy link
Contributor

Ms2ger commented Jul 10, 2015

Test doesn't seem like it can work, you're calling ttest rather than test.
@dzbarsky dzbarsky force-pushed the dzbarsky:master branch from d497e24 to 6b369a2 Jul 10, 2015
@dzbarsky
Copy link
Member Author

dzbarsky commented Jul 10, 2015

Hmm, I did check that the test failed before the change and passed after. I guess I somehow added the stray character after.
@dzbarsky
Copy link
Member Author

dzbarsky commented Jul 10, 2015

But the test doesn't finish now...need to investigate.
@dzbarsky dzbarsky force-pushed the dzbarsky:master branch from 6b369a2 to 0fa8e51 Jul 11, 2015
@dzbarsky
Test element prefix for element equality
Loading status checks…
0fa8e51
@dzbarsky
Copy link
Member Author

dzbarsky commented Jul 11, 2015

OK the relevant part of the test passes. I think something else may have regressed in the last few days causing the later part of the test to time out. We should enable the test and investigate in a followup.
@Ms2ger
Copy link
Contributor

Ms2ger commented Jul 13, 2015

@bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Jul 13, 2015

📌 Commit 0fa8e51 has been approved by Ms2ger
@bors-servo
Copy link
Contributor

bors-servo commented Jul 13, 2015

⌛ Testing commit 0fa8e51 with merge 4cc0d2d...
bors-servo pushed a commit that referenced this pull request Jul 13, 2015
bors-servo
Auto merge of #6554 - dzbarsky:master, r=Ms2ger
4cc0d2d 
Test namespace prefix for element equality



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

bors-servo commented Jul 13, 2015

💔 Test failed - mac3
@dzbarsky
Copy link
Member Author

dzbarsky commented Jul 13, 2015

Hmm that test is completely unrelated and unexpectedly passes for me even without this change
@dzbarsky
Copy link
Member Author

dzbarsky commented Jul 14, 2015

@bors-servo retry
@Ms2ger
Copy link
Contributor

Ms2ger commented Jul 14, 2015

@bors-servo retry
@bors-servo
Copy link
Contributor

bors-servo commented Jul 14, 2015

⌛ Testing commit 0fa8e51 with merge 91ce002...
bors-servo pushed a commit that referenced this pull request Jul 14, 2015
bors-servo
Auto merge of #6554 - dzbarsky:master, r=Ms2ger
Loading status checks…
91ce002 
Test namespace prefix for element equality



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

bors-servo commented Jul 14, 2015

☀️ Test successful - android, gonk, linux1, linux2, linux3, mac1, mac2, mac3
@bors-servo bors-servo merged commit 0fa8e51 into servo:master Jul 14, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Ms2ger Ms2ger

Labels
S-awaiting-review S-blocked-on-external
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
@dzbarsky @highfive @hoppipolla-critic-bot @Ms2ger @bors-servo @jdm
You can’t perform that action at this time.