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

platform/test: More test cases for receiver sets etc. #124

Merged
merged 1 commit into from Dec 25, 2016

Conversation

@antrik
Copy link
Contributor

antrik commented Nov 14, 2016

This adds a bunch of new test cases improving coverage for
OsIpcReceiverSet; along with a pair of tests for sending empty
messages in general, which wasn't covered before at all.

@antrik
Copy link
Contributor Author

antrik commented Nov 14, 2016

This is mostly to establish a baseline for #123

@antrik
Copy link
Contributor Author

antrik commented Nov 14, 2016

Oh, now that's an interesting catch... Though not exactly what I was after ;-)

@antrik antrik force-pushed the antrik:tests-receiver_set-etc branch 2 times, most recently from 9f5e8b0 to b472513 Nov 14, 2016
@antrik antrik force-pushed the antrik:tests-receiver_set-etc branch 2 times, most recently from 163efcf to 8390a33 Nov 16, 2016
@antrik
Copy link
Contributor Author

antrik commented Nov 16, 2016

I disabled the failing test on MacOS, since it can be argued that it doesn't expose a user-visible bug... Though the failure is still suspicious.

This adds a bunch of new test cases improving coverage for
`OsIpcReceiverSet`; along with a pair of tests for sending empty
messages in general, which wasn't covered before at all.
@antrik antrik force-pushed the antrik:tests-receiver_set-etc branch from 8390a33 to 310a956 Nov 18, 2016
@emilio
Copy link
Member

emilio commented Dec 25, 2016

Sounds good to me. Sorry this took so long to get reviewed @antrik, feel free to ping me if it happens again. I don't feel I have the time to do all the reviews I should in ipc-channel, but I'll try to find/bug an appropriate reviewer when possible.

Thanks again!

@bors-servo r+

@bors-servo
Copy link
Contributor

bors-servo commented Dec 25, 2016

📌 Commit 310a956 has been approved by emilio

@bors-servo
Copy link
Contributor

bors-servo commented Dec 25, 2016

Testing commit 310a956 with merge b8b2a76...

bors-servo added a commit that referenced this pull request Dec 25, 2016
platform/test: More test cases for receiver sets etc.

This adds a bunch of new test cases improving coverage for
`OsIpcReceiverSet`; along with a pair of tests for sending empty
messages in general, which wasn't covered before at all.
@bors-servo
Copy link
Contributor

bors-servo commented Dec 25, 2016

☀️ Test successful - status-appveyor, status-travis

@bors-servo bors-servo merged commit 310a956 into servo:master Dec 25, 2016
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@antrik
Copy link
Contributor Author

antrik commented Dec 25, 2016

@emilio thanks :-)

I wish this repository had highfive, or some other measures to make sure PRs are taken care of... I hate having to prod people all the time; and I find the difficulty of landing things in ipc-channel sometimes quite demotivating :-(

@fitzgen
Copy link
Member

fitzgen commented Dec 29, 2016

I wish this repository had highfive, or some other measures to make sure PRs are taken care of... I hate having to prod people all the time; and I find the difficulty of landing things in ipc-channel sometimes quite demotivating :-(

What is needed to get high five enabled for this repo? What you describe is a situation we absolutely shouldn't be in.

@fitzgen
Copy link
Member

fitzgen commented Dec 29, 2016

https://github.com/servo/highfive/#enabling-a-repo has steps to enable, although I don't have admin rights for this repo.

@metajack
Copy link

metajack commented Dec 29, 2016

Highfive webhook has now been added to this repo.

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.