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

Update webrtc and improve test consistency #211

Merged
merged 2 commits into from
Jun 29, 2018
Merged

Conversation

jacobheun
Copy link
Contributor

@jacobheun jacobheun commented Jun 28, 2018

Webrtc update

This should resolve #204 and #194.

The latest node-webrtc, 0.1.6, fixed an issue that was causing wrtc to fail.

I've verified this locally, and it looked good on the initial build of jenkins aside from an out of space issue. Jenkins is not having a good day/evening. I will re-run the build tomorrow so we get a clean green.

Test stability

There was an issue where running libp2p-mdns on jenkins could end up giving us wrong CIDs because the network is shared between tests. This adds a random serviceTag instead of using the default, which isolates the multicastDNS queries to the specific tests. This should prevent flakiness in those tests.

@ghost ghost assigned jacobheun Jun 28, 2018
@ghost ghost added the status/in-progress In progress label Jun 28, 2018
@jacobheun jacobheun changed the title chore: update latest webrtc Update webrtc and improve test consistency Jun 29, 2018
@jacobheun jacobheun requested a review from daviddias June 29, 2018 09:14
@jacobheun
Copy link
Contributor Author

@diasdavid I added a fix for test flakiness to this as well so we shouldn't see intermittent multicastDNS test failures anymore.

@@ -73,7 +73,7 @@
"pull-serializer": "~0.3.2",
"pull-stream": "^3.6.8",
"sinon": "^5.0.7",
"wrtc": "0.1.1"
"wrtc": "~0.1.6"
Copy link
Member

Choose a reason for hiding this comment

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

Wooot! :D

Copy link
Member

@daviddias daviddias left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @jacobheun :)

@daviddias daviddias merged commit 501cc22 into master Jun 29, 2018
@daviddias daviddias deleted the chore/update-wrtc branch June 29, 2018 18:38
@ghost ghost removed the status/in-progress In progress label Jun 29, 2018
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.

Cannot npm install with Node.js 10 -> Missing wrtc builds for Node.js 10
2 participants