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

Add appveyor.yml for automated Windows testing #602

Merged
merged 1 commit into from Feb 8, 2016

Conversation

@josephfrazier
Copy link
Member

josephfrazier commented Feb 8, 2016

#429 reminded me that webtorrent doesn't have any automated Windows testing. AppVeyor is a service that can provide that for us. Here's an example build: https://ci.appveyor.com/project/josephfrazier/webtorrent/build/1.0.6

Note that simply merging this isn't quite enough to get builds running. AppVeyor also needs to be configured here. I should be able to do this if/once this is merged.

feross added a commit that referenced this pull request Feb 8, 2016
Add appveyor.yml for automated Windows testing
@feross feross merged commit aa8c10c into webtorrent:master Feb 8, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@feross

This comment has been minimized.

Copy link
Member

feross commented Feb 8, 2016

Nice, hopefully this will help us catch when there's a regression on Windows! Much appreciated @josephfrazier!

@feross

This comment has been minimized.

Copy link
Member

feross commented Feb 8, 2016

It looks like it's all configured, if you go here: https://ci.appveyor.com/project/feross/webtorrent

Also, added a badge to the readme :)

@josephfrazier

This comment has been minimized.

Copy link
Member Author

josephfrazier commented Feb 9, 2016

Sweet, thanks! There's a noticeable delay on the builds, but they're better late than never.

One thing that I thought you might be interested in is that the node_modules directory is cached for the AppVeyor builds. I had done the same for Travis builds, but that change was undone (for reasons I didn't quite agree with). Do you want to remove the caching here as well?

@josephfrazier josephfrazier deleted the josephfrazier:appveyor2 branch Feb 9, 2016
@feross

This comment has been minimized.

Copy link
Member

feross commented Feb 9, 2016

@josephfrazier Just saw your comment - sorry for missing it before. I think you're generally right. We should update package.json when a newer version is required for webtorrent to work correctly. Let's try to do that going forward 👍

That said, I prefer not to cache the node_modules folder on CI since I want to catch bugs that exist in the latest published packages, i.e. what a user will experience when running npm install webtorrent for the first time.

@feross

This comment has been minimized.

Copy link
Member

feross commented Feb 9, 2016

For example, if we fix a bug and add a test for it, but forget to update package.json to be greater than the buggy version, that's bad (since users could still have the bug), but not as bad as if npm install webtorrent gets completely broken by a bad patch version of a dependency. I care more about catching the latter. :)

@josephfrazier

This comment has been minimized.

Copy link
Member Author

josephfrazier commented Feb 9, 2016

Oh, that makes sense now, given that the new user experience is prioritized. I'll update appveyor.yml accordingly. Thanks for the explanation!

josephfrazier added a commit to josephfrazier/webtorrent that referenced this pull request Feb 9, 2016
See the discussion at
webtorrent#602 (comment)

> I prefer not to cache the node_modules folder on CI since I want to
> catch bugs that exist in the latest published packages, i.e.  what a
> user will experience when running npm install webtorrent for the first
> time.
josephfrazier referenced this pull request Feb 9, 2016
It’s causing the tests to fail because there’s an old version of
create-torrent being used.
@feross

This comment has been minimized.

Copy link
Member

feross commented Feb 9, 2016

@josephfrazier Thanks for caring so much about testing and reliability! :)

@lock

This comment has been minimized.

Copy link

lock bot commented May 4, 2018

This thread has been automatically locked because it has not had recent activity. To discuss futher, please open a new issue.

@lock lock bot locked as resolved and limited conversation to collaborators May 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Linked issues

Successfully merging this pull request may close these issues.

None yet

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