Skip to content
This repository was archived by the owner on Jul 28, 2024. It is now read-only.

Have Travis run tests#1415

Merged
borekb merged 3 commits intoversionpress:masterfrom
candrews:improve-travis
Apr 5, 2019
Merged

Have Travis run tests#1415
borekb merged 3 commits intoversionpress:masterfrom
candrews:improve-travis

Conversation

@candrews
Copy link
Copy Markdown
Contributor

@candrews candrews commented Mar 26, 2019

Issue: #1259

Travis will now run markdownlint, PHP CodeSniffer, build the Docker images and run the full tests.

With this change, VersionPress can now only merge PRs that have passing tests, ensuring that all tests always pass.

(This is a similar PR to #1381, after we have fixed the testing infrastructure in #1389 so that the tests actually pass.)

Note that the Docker caching is not part of this PR and should be done separately, see #1419.

@borekb

This comment has been minimized.

@borekb
Copy link
Copy Markdown
Member

borekb commented Mar 26, 2019

Have you seen the updated description of #1259? While the tests run much faster now, it's still wasteful to run long builds for commits that didn't change any code.

@candrews
Copy link
Copy Markdown
Contributor Author

IMHO, Travis currently takes 15 minutes to run the whole thing, which isn't that long. It's not an ideal allocation of compute resources, but I don't see the harm - a few extra occurrences of 15 minute builds isn't going to really impact global warming, and since the builders are free, it's not going to break the bank either :)

Plus, if you run full tests for each PR, you always know the full tests pass. If you get "fancy" and try to run a subset, it's possible to make a mistake and not run a test that should have been run due an error in the test harness. I think KISS is the way to go.

In this PR, the tests also fast fail (that's due to the set -e in the scripts section) so if there's a failure, it'll be reported as quickly as possible and the rest of the steps don't run. I also ordered it so the fastest stuff is first.

@borekb
Copy link
Copy Markdown
Member

borekb commented Mar 26, 2019

Ok, let's do it for now. If we want to improve this in the future, here are some links:

This example is enough to get an idea. Click here to see more links.

This is the upstream issue: travis-ci/travis-ci#6301.

One thing I currently don't like is that Docker images are re-built every time – looks like the layer caching is missing. This is not only inefficient but also produces a lot of noise in Travis logs. I'm not sure how Docker caches this but if this could be added to cache, it would be great.

@borekb
Copy link
Copy Markdown
Member

borekb commented Mar 26, 2019

Overall, it's awesome to see the test passing on a CI! 🤖

@borekb borekb added scope: dev-infrastructure Build scripts, IDE settings, CI, Docker dev stack, testing, tooling, etc. scope: tests Testing code. For infrastructure (CI, etc.), use "dev-infrastructure". labels Mar 26, 2019
@borekb borekb added this to the 4.0 milestone Mar 26, 2019
This was referenced Mar 26, 2019
@borekb
Copy link
Copy Markdown
Member

borekb commented Mar 27, 2019

Thanks for the changes, I'll take a look shortly.

I'd just like to ask for "normal commits" in the future, not force-pushes. For example, the .travis.yml evolved over several commits which is not evident from the history of this PR, and some of the links are broken:

Screenshot 2019-03-27 at 15 49 06

Also, the iis.md formatting fix was first here, then it wasn't (the first force-push), now it's back again (maybe a mistake during the second force-push?), it's all quite confusing.

We prefer multiple smaller commits, some of them heavy WIP, over "clean" (but fake) history.

@candrews
Copy link
Copy Markdown
Contributor Author

We prefer multiple smaller commits, some of them heavy WIP, over "clean" (but fake) history.

No problem - different projects have different requirements, and I've recently been working on ones that use a different model :)

@borekb
Copy link
Copy Markdown
Member

borekb commented Mar 27, 2019

Thank you, it's not documented very well right now, our fault ;)

@borekb borekb merged commit 62bf671 into versionpress:master Apr 5, 2019
@borekb
Copy link
Copy Markdown
Member

borekb commented Apr 5, 2019

Thanks, @candrews! I've created a separate issue for Docker caching: #1419.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

scope: dev-infrastructure Build scripts, IDE settings, CI, Docker dev stack, testing, tooling, etc. scope: tests Testing code. For infrastructure (CI, etc.), use "dev-infrastructure".

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants