Skip to content

Update Travis CI configuration#2408

Merged
westonruter merged 26 commits intodevelopfrom
update/2384-testing
May 27, 2019
Merged

Update Travis CI configuration#2408
westonruter merged 26 commits intodevelopfrom
update/2384-testing

Conversation

@swissspidy
Copy link
Copy Markdown
Collaborator

@swissspidy swissspidy commented May 24, 2019 •
edited
Loading

What this PR does:

  • First and foremost, this runs JavaScript unit tests on Travis CI in a separate job.
  • Disables branch builds for pull requests.
    Travis already runs for PRs itself, running it again for the merge commit is usually pointless and just takes time.
  • Creates a separate build stage for linting only.
    This way, we don't bother running tests when code isn't properly formatted.
  • Lints composer.json and package.json files in this new lint stage.
  • Turns on fast finishing.
    This way, the build will be marked as finished as soon as one job has failed. This is done under the assumption that all the other tests are likely to fail too when one has failed already.
  • Disables redundant PHP syntax and ESLint checks.
    ESLint is already run during the lint stage, and PHP syntax only needs to be tested once per PHP version.
  • Reverse order of jobs to test on newer PHP and WordPress versions first.
    Together with the fast finishing this makes things a bit faster in case of failures.
  • Adds some names to the different jobs to make it clearer what each job does.
    See screenshots below for comparison.
  • Only installs PWA plugin on WP 5.2+ and PHP 5.6+
  • Runs tests against WordPress trunk again
    This job is allowed to fail
  • Makes tests more robust / fixes covers annotations

Before (link)

Screenshot 2019-05-24 at 12 36 42

After (link)

Screenshot 2019-05-24 at 12 36 50

Some thoughts/notes

  • npm run build:js should unnecessary for the linting stage and could probably be skipped there.
  • Continuous builds would be nice at some point. (see Reliable builds and deploys #1840)
  • Is that ampconfdemo deploy stage still relevant?
  • There are no tests against WP_VERSION=nightly.
    Changing that would be useful to catch potential conflicts early, and not just after WordPress has been released.
  • Always installing Gutenberg for WordPress 5.0+ seems excessive and dangerous.
    If I am not missing something, this never tests WordPress core's block editor integration as-is, but always uses the the Gutenberg one. Currently not much of a problem, as Gutenberg does not have any new PHP stuff at the moment. But when they're going to drop support for PHP < 5.6 this might cause problems in our tests.
  • Always installing the PWA plugin seems a bit excessive for just the few service worker tests there are.
    Given that the PWA plugin only supports WordPress 5.2+, this should only be installed when testing WordPress 5.2+.

Fixes #2384.

@googlebot googlebot added the cla: yes Signed the Google CLA label May 24, 2019
@swissspidy swissspidy changed the title Update travis configuration [WIP] Update travis configuration May 24, 2019
@swissspidy swissspidy marked this pull request as ready for review May 24, 2019 07:20
swissspidy added 4 commits May 24, 2019 10:50
@swissspidy
More config fixes
4635a45
@swissspidy
Explicityl skip xmllint in other stages as well
0d01c19
@swissspidy
Use rootDir in testPathIgnorePatterns
78a441e 
Prevents skipping legit files on Travis CI because `/build/` would also match `/home/travis/build/ampproject/amp-wp`
@swissspidy
Trigger build
24e4524
@swissspidy swissspidy added this to the v1.2 milestone May 24, 2019
@swissspidy swissspidy requested a review from westonruter May 24, 2019 10:46
@swissspidy swissspidy changed the title [WIP] Update travis configuration Update Travis CI configuration May 24, 2019
@westonruter
Copy link
Copy Markdown
Member

westonruter commented May 24, 2019

  • Is that ampconfdemo deploy stage still relevant?

This was being used for QA last year but it has slowly discontinued being used. I'd rather see the automatic deploys be made to *.amp-wp.org environments now.

  • There are no tests against WP_VERSION=nightly. Changing that would be useful to catch potential conflicts early, and not just after WordPress has been released.

We did have trunk until October when it was replaced with 5.0: af37192. This was done because of instability of core at the time, I believe. Perhaps trunk should be put in an allow_failures bucket.

  • Always installing Gutenberg for WordPress 5.0+ seems excessive and dangerous.
    If I am not missing something, this never tests WordPress core's block editor integration as-is, but always uses the the Gutenberg one. Currently not much of a problem, as Gutenberg does not have any new PHP stuff at the moment.

You're right. Making Gutenberg a dependency was specifically to facilitate testing with AMP Stories. Once AMP Stories no longer requires Gutenberg to work (which may be the case as of 5.2 actually) then we should only install/activate Gutenberg on a single env.

But when they're going to drop support for PHP < 5.6 this might cause problems in our tests.

We should also drop support for PHP<5.6 then as well.

  • Always installing the PWA plugin seems a bit excessive for just the few service worker tests there are.
    Given that the PWA plugin only supports WordPress 5.2+, this should only be installed when testing WordPress 5.2+.

Makes sense to me.

westonruter
westonruter requested changes May 24, 2019
View reviewed changes
swissspidy added 15 commits May 27, 2019 11:54
@swissspidy
Merge branch 'develop' into update/2384-testing
5fb6fb6
@swissspidy
Experiment with non-separated lint stage
310cf15 
Should be faster because of parallel jobs and fast finishing
@swissspidy
Run against release branches too
0a581c9
@swissspidy
Simplify config by using DEV_LIB_ONLY
ccceddb
@swissspidy
Test against WP trunk in allow_failures group
f3ab24d
@swissspidy
Only install PWA plugin on WP 5.2+ and PHP 5.6+
8a81af8
@swissspidy
Fix indentation in travis config
ae2be93
@swissspidy
Don’t build JS when not needed
8f7cc22
@swissspidy
Mark tests as skipped, not incomplete, when PWA plugin is missing
ee07b0a
@swissspidy
Also validate package.json and composer.json during lint
6db53a3
@swissspidy
Make lint stage separate again
de70e37
@swissspidy
Try moving allow_failures section up
c664b5b
@swissspidy
Try using separate stage for allowed failures jobs
50bd8fc
@swissspidy
Add comment about stylelint
49bfedf
@swissspidy
Skip stylesheet priorization test if twentyten is not installed
ba891ce
@swissspidy swissspidy mentioned this pull request May 27, 2019
Closed
@swissspidy swissspidy requested a review from westonruter May 27, 2019 13:09
@swissspidy
Copy link
Copy Markdown
Collaborator Author

swissspidy commented May 27, 2019

Note: I was a bit surprised to see some of the tests pass. \AMP_Style_Sanitizer_Test::test_prioritized_stylesheets failed locally because it requires the Twenty Ten theme to be installed.

Turns out, wp-dev-lib downloads WP latest from https://develop.svn.wordpress.org/tags/5.2.1/, and bin/install-wp-tests.sh does so from https://wordpress.org/wordpress-5.2.1.zip. The former includes all themes, but the latter does only include the Themes 2016, 2017, and 2019. Hence the failure.

In ba891ce I skipped the test if the theme is missing.

@westonruter westonruter merged commit 802cd13 into develop May 27, 2019
@westonruter westonruter deleted the update/2384-testing branch May 27, 2019 15:38
@swissspidy swissspidy mentioned this pull request May 27, 2019
Merged
@westonruter westonruter mentioned this pull request May 28, 2019
Merged
@swissspidy swissspidy mentioned this pull request Jan 9, 2020
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@westonruter westonruter westonruter approved these changes

Assignees

No one assigned

Labels

cla: yes Signed the Google CLA

Projects

None yet

Milestone

v1.2

Development

Successfully merging this pull request may close these issues.

Run JavaScript Unit Tests on Travis

3 participants

@swissspidy @westonruter @googlebot