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

Proposal: Adjust ci jobs to actual pr changes #4384

Open
ezyang opened this issue Mar 9, 2017 · 11 comments
Open

Proposal: Adjust ci jobs to actual pr changes #4384

ezyang opened this issue Mar 9, 2017 · 11 comments

Comments

@ezyang
Copy link
Contributor

ezyang commented Mar 9, 2017

At the moment, we run builds under 14 different environments when we do CI. I have noticed that for the vast majority of commits, this is wasted effort. Usually, the important things are to (1) test every GHC version, (2) test Windows, and (3) test Mac OS X.

So, I propose to reduce the set of configurations we test during PRs to Linux (once for every GHC version), Windows, and the latest GHC with Mac OS X. We'll continue to run all tests for commits to master, and we should have some mechanism for triggering the extra tests in PR if a user wants them (e.g., a high risk commit.)

@23Skidoo
Copy link
Member

23Skidoo commented Mar 9, 2017

Makes sense. Windows code sometimes breaks on older versions of GHC because of CPP, but we use only one GHC version on AppVeyor anyway (would be nice if we included more versions as part of the full check on master).

@ezyang
Copy link
Contributor Author

ezyang commented Mar 10, 2017

Ugh, apparently, Travis can't actually do this directly.

@DanielG
Copy link
Collaborator

DanielG commented Mar 10, 2017

You could use $TRAVIS_PULL_REQUEST to just short circuit the matrix elements you don't want I guess?

@ezyang
Copy link
Contributor Author

ezyang commented Mar 10, 2017

Yeah. It's annoying though because we'll still end up with matrix entries (and pay the spin up costs) of every matrix entry we don't end up using.

@23Skidoo
Copy link
Member

@ezyang Is there a relevant ticket on the Travis issue tracker? Perhaps we should create one.

@ezyang
Copy link
Contributor Author

ezyang commented Mar 11, 2017

I think this one is the closest: travis-ci/travis-ci#2778

@ezyang ezyang added this to the milestone Mar 23, 2017
@robertpeteuil
Copy link

robertpeteuil commented Apr 20, 2017

I agree that paying the spin-up-cost for matrix entries we don't need is annoying.

@dwijnand dwijnand mentioned this issue Oct 27, 2019
4 tasks
@andreabedini
Copy link
Collaborator

Not sure this issue is still relevant but as of #7952 we run tests against 9 versions of GHC on Linux and MacOS, and two on Windows. Plus other tests (dogfooding, quick checks).

I believe it's a lot.

@jneira
Copy link
Member

jneira commented Feb 22, 2022

Ideally we should adjust ci matrix to pr actual changes. You could for example, only run Cabal tests for only Cabal changes, no run tests for only docs changes, etc. But for big prs touching several components it would be good to run as much configurations as possible, in a automated way.

In hls we have used cancel-action to run different tests depending on file globs, not perfect but it saves time and resources:

https://github.com/haskell/haskell-language-server/blob/3084651ba467c3306ffd7d1024fbd3a13a64a296/.github/workflows/test.yml#L21-L53

@andreabedini
Copy link
Collaborator

@jneira yes, that's what I was going at in #8000 (comment).

I think we can close this issue, unless you want to keep it for discussion and that's fine for me.

@jneira
Copy link
Member

jneira commented Feb 22, 2022

I think we can close this issue, unless you want to keep it for discussion and that's fine for me.

Well we can keep open to remember us we can improve the ci configuration to adjust ci work to actual code changes.

@jneira jneira changed the title Proposal: Reduce CI matrix for PRs Proposal: Adjust ci jobs to actual pr changes Mar 11, 2022
@andreabedini andreabedini added the old-milestone: ⊥ Moved from https://github.com/haskell/cabal/milestone/5 label Oct 17, 2023
@ulysses4ever ulysses4ever removed this from the milestone Dec 15, 2024
@ulysses4ever ulysses4ever removed the old-milestone: ⊥ Moved from https://github.com/haskell/cabal/milestone/5 label Dec 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants