Skip to content

Let's just do one AppVeyor run instead of four #3238

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

Closed
gvanrossum opened this issue Apr 24, 2017 · 9 comments
Closed

Let's just do one AppVeyor run instead of four #3238

gvanrossum opened this issue Apr 24, 2017 · 9 comments

Comments

@gvanrossum
Copy link
Member

I don't think the cost of waiting for all four runs is worth the benefits. There is no different mypy functionality being tested -- at best we'll catch 32/64 bugs in CPython or typed_ast.

@ddfisher Do you have any objections? Do the four AppVeyor runs contribute to the Windows wheel building?

@JukkaL
Copy link
Collaborator

JukkaL commented Apr 25, 2017

If they are needed only for wheel building, can we move them to a separate job that gets run on demand somehow? At least we should be able to do something like this:

  • Set up another repo that just contains stuff needed for AppVeyor and a mypy commit pin.
  • The AppVeyor build for that new repo just clones the mypy repo and runs mypy tests (and builds the wheels that we may need).
  • To manually trigger a build, just push a commit to the new repo that updates the mypy pin.
  • Give all of mypy core team commit access to the new repo.

@ddfisher
Copy link
Collaborator

We do use the Appveyor builds for Windows wheels, but we only need two of them for that -- a 32 bit and a 64 bit build. The Python version doesn't matter. I think we can make one of the builds only run on tagged commits if we want.

Can we try switching to 2 Appveryor builds first and see if that helps much? That'd be a very easy change to start with.

@gvanrossum
Copy link
Member Author

Well, it'll be twice as slow (as with only one build) but still twice as fast (compared to the current situation with 4 boulds).

@pkch
Copy link
Contributor

pkch commented Apr 25, 2017

Also, currently every push to a branch on the main repo (not a fork) causes AppVeyor to build both the branch commit, and the hypothetical merge-with-master commit. I'm not saying it's useless, but maybe we can live with just "merge-with-master"? If a branch can't be merged cleanly with tests passing, it probably results in the same effort to fix it as if the branch itself failed a test.

AppVeyor has a web UI setting to just do the merge builds: Do not build feature branches with open Pull Requests; it's equivalent to setting skip_branch_with_pr: true in appveyor.yml.

In addition, we might consider disabling builds on master by using:

except:
  - master 

since commits to master are usually not done except by merging a PR. (It's always possible to initiate a build manually if desired.)

@pkch
Copy link
Contributor

pkch commented Apr 25, 2017

The last point (disabling master tests) is safe only if we use github setting Require branches to be up to date before merging. If not, we should keep building on each master commit, since the PR build may be obsolete by the time the merge happens.

@gvanrossum
Copy link
Member Author

I thought the duplicate test runs were only an issue for core devs?

@pkch
Copy link
Contributor

pkch commented Apr 26, 2017

They use AppVeyor capacity allocated to python/mypy repo. Below I copied the screenshot of the build history from AppVeyor for mypy project from a minute ago. I highlighted in yellow the pairs of builds where one would kinda suffice. Most of them completed in full; a couple of them were cancelled, but only due to another push to the same PR so that our "Rolling builds" setting could kill them half-way:

appveyor_mypy

@pkch
Copy link
Contributor

pkch commented Apr 26, 2017

To clarify: in this picture, there can be only 1 build running at any given moment, since we only have 1 VM. Anything else (that's not cancelled) will be queued up.

@gvanrossum
Copy link
Member Author

Hm, I recall researching a similar think for Travis CI and ending up thinking it was too complicated.

If you want to change something please open a new issue and state clearly what the instructions are and what the risks are. (And the benefits for good measure, though I think we all understand those. :-)

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

No branches or pull requests

4 participants