Silverstripe-framework: RFC: [META] Unify travis config across modules

Created on 12 Aug 2019  ยท  24Comments  ยท  Source: silverstripe/silverstripe-framework

WARNING

This RFC was created on 12 Aug 2019. Since then TravisCI introduced config imports feature, which provides built-in means for reusing configs across different repositories, including build matrices.

Read more details in the comments below


Outline

Currently we use Travis to run our automated tests (as everyone is probably aware). We have a sort of unwritten rule that we use one distro, a bunch of different supported PHP versions and probably a select few different core releases for the test matrix on travis across all ~100 of the modules that we still maintain in the open source community.

Here are the problems:

  • The Travis scripts are not consistent
  • When core releases are made - all travis configs need to be updated across all modules
  • If Travis changes their expectations, all configs need to be updated across all modules
  • If travis configs are not updated - we don't necessarily realise until a community member makes a PR and all the builds fail (and it's not related to their contribution)

Proposal

We introduce a "one travis config to rule them all". Unfortunately Travis doesn't support any way to share a config across repos but having the same config across repos means that we can feel safe updating the config - and possibly create an automated system to do it.

There's a few variables per repo though. Modules may have:

  • behat tests
  • javascript tests
  • coverage
  • dependencies on newer versions of core modules only

This means there must be _some_ variance in the matrix for each repo, and that can't be dynamic.

Ideas:

1) We have a shared travis config that still needs some editing, but we implement some features to avoid having to update it as often.

Outline:

  • The config will have consistent distro settings
  • Modules will optionally (and explicitly) include "addons" they need for behat
  • Modules will edit "env" vars for node version and minimum core dependency
  • We don't define specific core versions in the matrix but that's assigned by using a script like what @Cheddam whipped up here: https://gist.github.com/Cheddam/f3c8df2e097c66a7c8a7242f2c6b3c96

Pros:

  • We don't have to update all configs when we do core releases

Cons:

  • We still have to manually edit travis configs when Travis changes

Notes:

  • We might possibly be able to automate updating the script in the future provided we're strict about what we can edit

2) We automate travis configs with some script:

Outline:

  • We parse the composer file or something else to determine whether JS/Behat tests are necessary
  • The script generates a travis config and diffs it against the current config (for each repo)
  • The script raises a PR if there are changes

Pros:

  • No manual update of travis config
  • Everythings always in sync
  • Moving to a different CI tool might be easier in the future? ๐Ÿค”

Cons:

  • Effort
  • We have more "scaffolding" in the form of config files (config to generate config)

Notes:

  • We could still use something like @Cheddam's script above to reduce the number of changes to config over time.
rfaccepted

Most helpful comment

_inhales extremely deeply_

The anatomy of a .travis.yml file

Let's break down the structure of a Travis configuration file, to see what can be shared across every module (โœ…), and what needs to generated with per-module/branch configuration (โš™๏ธ). The below examples are largely pulled from dnadesign/silverstripe-elemental.

Note that we're talking about 'satellite modules' here, as they vastly outnumber core modules and are therefore more important to unify. Core modules will need slightly different handling, which we can tackle once the approach for satellite modules has been implemented and proven viable.

Distro / Packages โœ…

Some basic declarations about what we need installed:

language: php

dist: xenial

services:
  - mysql
  - postgresql
  - xvfb

addons:
  apt:
    packages:
      - tidy
      - chromium-chromedriver
      - chromium-browser

This section can be standardised across all of our modules, _provided we install xvfb/chromium regardless of whether Behat tests are present_. This will require infrequent changes to support new distros.

Environment Variables โš™๏ธ

This defines global environment variables that need to be present in every build.

env:
  global:
    - COMPOSER_ROOT_VERSION=4.3.x-dev
    - TRAVIS_NODE_VERSION="10"
    - DISPLAY=":99"
    - XVFBARGS=":99 -ac -screen 0 1024x768x16"
    - SS_BASE_URL="http://localhost:8080/"
    - SS_ENVIRONMENT_TYPE="dev"

Modules without Behat testing generally only have COMPOSER_ROOT_VERSION set in this section. This variable is different for every branch of a module, and is crucial for certain circular dependency resolution situations. It should be possible to generate this based on the branch name, rather than requiring a way to set custom environment variables per module.

TRAVIS_NODE_VERSION isn't necessary, since we can run nvm use to set the correct Node version based on the contents of .nvmrc. (We might need to add this file to a few modules that are missing it, though.)

Build Matrix โš™๏ธ

This section defines what builds will be run. (NOTE: I actually lifted this from another module, as Elemental 4.3's matrix was in a bit of a weird state.)

matrix:
  include:
    - php: 7.1
      env: DB=PGSQL RECIPE_VERSION=4.3.x-dev PHPCS_TEST=1 PHPUNIT_TEST=1
    - php: 7.2
      env: DB=MYSQL RECIPE_VERSION=4.4.x-dev PHPUNIT_TEST=1
    - php: 7.3
      env: DB=MYSQL RECIPE_VERSION=4.4.x-dev PHPUNIT_COVERAGE_TEST=1
    - php: 7.3
      env: DB=MYSQL RECIPE_VERSION=4.4.x-dev NPM_TEST=1
    - php: 7.3
      env: DB=MYSQL RECIPE_VERSION=4.x-dev PHPUNIT_TEST=1

Each build defines a PHP version to use, and sets environment variables:

  • DB=MYSQL/PGSQL is effectively a boolean that defines whether the PostgreSQL support module will be installed. Some modules don't officially support PostgreSQL, so they lack builds with the PGSQL value.
  • RECIPE_VERSION=4.x-dev defines which version of core will be installed, generally via silverstripe/recipe-core or silverstripe/recipe-cms.

    • At minimum, each of the currently supported core versions and the next version (4.x) should be covered in these builds.

    • We generally prefer development branches to stable tags as they can occasionally catch issues in core before they ship. However, when the development branch of a minor version is removed, we have to fall back to tags if we still want to test them (currently 4.0-4.2 are impacted by this).

  • PHPUNIT_TEST / PHPUNIT_COVERAGE_TEST, PHPCS_TEST, BEHAT_TEST, NPM_TEST: These designate what type of tests will be run. PHPUnit and Behat tests should be run for every PHP and core version, whilst the other test types should only be run once.

The build matrix can't be made identical across all modules or branches, due to a lot of variations:

  • Different minimum core support
  • Different minimum PHP support
  • Some builds install extra modules for cross-module support testing
  • Some modules don't have Behat / JS tests

We can use configuration to generate a matrix based on these variations, with the following tactics:

  • We could reference composer.json for the branch to determine the core / PHP versions that can be tested.
  • For cross-module testing, we could define a custom section in composer.json called require-test, or simply ensure that the extra modules are always installed in the before_script stage via custom config.
  • We'll need to define clear configuration for each module as to whether they have Behat / JS tests to run. It might be possible to infer these based on existing files / config, but I'm not confident this would be reliable.

Regarding --prefer-latest and --prefer-lowest, I think employing the latter to warn us when our core requirements are too low makes sense, but for supported minor core releases we should continue to test each one explicitly to catch UI breakages and usage of APIs that aren't present in prior releases.

Pre-build scripts โœ…

These handle installing dependencies and spinning up required services.

before_script:
  - sudo apt-get remove -y --purge google-chrome-stable || true

  # Init PHP
  - phpenv rehash
  - phpenv config-rm xdebug.ini
  - echo 'memory_limit = 2G' >> ~/.phpenv/versions/$(phpenv version-name)/etc/conf.d/travis.ini

  # Install composer dependencies
  - composer validate
  - composer require silverstripe/recipe-cms:"$RECIPE_VERSION" silverstripe/recipe-testing:^1 --no-update
  - if [[ $BEHAT_TEST ]]; then composer require --no-update silverstripe/behat-extension:4.x-dev; fi;
  - if [[ $DB == PGSQL ]]; then composer require silverstripe/postgresql:^2 --no-update; fi
  - composer install --prefer-source --no-interaction --no-progress --no-suggest --optimize-autoloader --verbose --profile

  # Behat bootstrapping
  - if [[ $BEHAT_TEST ]]; then mkdir artifacts; fi
  - if [[ $BEHAT_TEST ]]; then cp composer.lock artifacts/; fi
  - if [[ $BEHAT_TEST ]]; then (chromedriver > artifacts/chromedriver.log 2>&1 &); fi
  - if [[ $BEHAT_TEST ]]; then (vendor/bin/serve --bootstrap-file vendor/silverstripe/cms/tests/behat/serve-bootstrap.php &> artifacts/serve.log &); fi

There are some common patterns across our modules in before_script, but they're often implemented slightly differently. The core set of steps that have to be run:

  • Purge conflicting Chrome install (a Xenial-specific nuance, this might be removed in the future)
  • Configure PHP (disable XDebug and bump the memory limit)
  • Add Composer requirement for specified core recipe
  • Conditionally add Composer requirements for PostgreSQL and Behat modules
  • Validate / install Composer requirements
  • Bootstrap Behat if required
  • Install NPM dependencies if required

I _think_ we could come up with a standardised config here, but we'll need to expose a method of prepending / appending additional steps for modules that have really unique scenarios. These hooks could be implemented via Composer scripts (e.g. travis-prepend-before-script and travis-append-before-script).

Build scripts โœ…

These are the actions that actually run our tests.

script:
  - if [[ $PHPUNIT_TEST ]]; then vendor/bin/phpunit tests/; fi
  - if [[ $PHPUNIT_COVERAGE_TEST ]]; then phpdbg -qrr vendor/bin/phpunit --coverage-clover=coverage.xml; fi
  - if [[ $PHPCS_TEST ]]; then vendor/bin/phpcs src/ tests/ ; fi
  - if [[ $NPM_TEST ]]; then git diff-files --quiet -w --relative=client; fi
  - if [[ $NPM_TEST ]]; then git diff --name-status --relative=client; fi
  - if [[ $NPM_TEST ]]; then yarn run test; fi
  - if [[ $NPM_TEST ]]; then yarn run lint; fi
  - if [[ $BEHAT_TEST ]]; then vendor/bin/behat @silverstripe-elemental; fi

These are _fairly_ standard already, so I think this could be module-agnostic. The only reference to the specific module in that list is the Behat suite name, which we could potentially shift into a per-module global environment variable (or similarly to the way NPM tests run, we could create a standard Composer script, e.g. composer run-script behat, and have the modules maintain the exact command that is run). We could also offer some hooks for adding custom config if necessary (as with the previous section, my suggestion would be appropriately named Composer scripts)

Post-build Scripts โœ…

These sections cover any required actions based on the outcome of the build.

after_success:
  - if [[ $PHPUNIT_COVERAGE_TEST ]]; then bash <(curl -s https://codecov.io/bash) -f coverage.xml; fi

after_failure:
- php ./vendor/silverstripe/framework/tests/behat/travis-upload-artifacts.php --if-env BEHAT_TEST,ARTIFACTS_BUCKET,ARTIFACTS_KEY,ARTIFACTS_SECRET --target-path $TRAVIS_REPO_SLUG/$TRAVIS_BUILD_ID/$TRAVIS_JOB_ID --artifacts-base-url https://s3.amazonaws.com/$ARTIFACTS_BUCKET/ --artifacts-path ./artifacts/

These aren't present across all modules at the moment (some don't have code coverage, others don't Behat tests) but they're safe enough to standardise. (We can also add some hooks to these steps if necessary.)

Automation

This feels like a job for a bot. We have access to GitHub Actions now, so we can trigger the bot on activities like:

  • Configuration files changing in a module
  • New branches being created
  • Shared Travis configuration changes

We can then feed it a list of modules to manage, some configuration around which core versions are currently supported, and a template to base its output on, and it can start raising PRs whenever changes are required. (We can probably even put it in charge of creating the GitHub Action configuration in each managed repo ๐Ÿ˜„)

The RFC

Per the official process, we need to get this out for a core committer vote in order to move forward with an implementation. @chillu, do you feel there's a sufficient amount of information present to go forward with a proof of concept?

All 24 comments

So in terms of frequency of changes:

  • Every ~3 months: Add new core minor release, remove minor releases which are now unsupported
  • Every ~6 to ~18 months : Add new PHP version, remove unsupported ones
  • Every ~12 to ~24 months: Switch to a new distro, respond to other Travis configuration or capability changes

Then there's other one off changes:

  • Introduce new tests (e.g. vulnerability or static analysis)
  • Making use of new opt-in Travis features (e.g. fast_finish)

So across ~100 modules, we're talking about ~600 travis config edits per year. Multiply that by more than one active release line, we're talking ~2000. That's a lot of time, even if it's just pressing merge buttons on auto generated pull requests.

Looking at how Travis build matrix works, I don't think we can generate this during build scheduling within Travis (e.g. by downloading currently supported versions via an API). We could run one job with multiple variations (e.g. phpunit on 4.3 and 4.4), but that makes our builds more complex, and build failures less obvious.

In terms of a "shared travis config", I don't know how that would work. Travis reads the .travis.yml file, and creates jobs based on the matrix it finds there. While we could pull in the bash commands in script blocks from an external source (e.g. a shared Github repo), we can't dynamically pull in the rest of the file.

Overall, I think we should invest into the second suggestion: Automating the creation of .travis.yml from a template, for a given branch and repo. We still need to solve on how we determine supported branches per module. For example, if we ran this tool against the blog module right now, it would create over a dozen pull requests. We might need to delete unmaintained branches after all? Or we add this metadata to https://github.com/silverstripe/supported-modules/blob/gh-pages/modules.json.

Note that Laravel runs two build variations: --prefer-latest and --prefer-lowest. https://github.com/laravel/framework/blob/6.x/.travis.yml#L39. That's pretty smart actually: If your silverstripe/blog constraint says silverstripe/framework:~4, then you could reason it should test against silverstripe/framework:4.0.0 when asking for --prefer-lowest. The fact that 4.0.0 isn't supported any more as an upstream dependency is kind of secondary. If there are incompatibilities in silverstripe/blog which require a newer framework version, it should up the dependency, e.g. to silverstripe/framework:~4.4. That means we wouldn't be running blog with older supported minor releases (e.g. silverstripe/framework:4.3.x), but it also means we'd never have to update the core dependences on modules.

_inhales extremely deeply_

The anatomy of a .travis.yml file

Let's break down the structure of a Travis configuration file, to see what can be shared across every module (โœ…), and what needs to generated with per-module/branch configuration (โš™๏ธ). The below examples are largely pulled from dnadesign/silverstripe-elemental.

Note that we're talking about 'satellite modules' here, as they vastly outnumber core modules and are therefore more important to unify. Core modules will need slightly different handling, which we can tackle once the approach for satellite modules has been implemented and proven viable.

Distro / Packages โœ…

Some basic declarations about what we need installed:

language: php

dist: xenial

services:
  - mysql
  - postgresql
  - xvfb

addons:
  apt:
    packages:
      - tidy
      - chromium-chromedriver
      - chromium-browser

This section can be standardised across all of our modules, _provided we install xvfb/chromium regardless of whether Behat tests are present_. This will require infrequent changes to support new distros.

Environment Variables โš™๏ธ

This defines global environment variables that need to be present in every build.

env:
  global:
    - COMPOSER_ROOT_VERSION=4.3.x-dev
    - TRAVIS_NODE_VERSION="10"
    - DISPLAY=":99"
    - XVFBARGS=":99 -ac -screen 0 1024x768x16"
    - SS_BASE_URL="http://localhost:8080/"
    - SS_ENVIRONMENT_TYPE="dev"

Modules without Behat testing generally only have COMPOSER_ROOT_VERSION set in this section. This variable is different for every branch of a module, and is crucial for certain circular dependency resolution situations. It should be possible to generate this based on the branch name, rather than requiring a way to set custom environment variables per module.

TRAVIS_NODE_VERSION isn't necessary, since we can run nvm use to set the correct Node version based on the contents of .nvmrc. (We might need to add this file to a few modules that are missing it, though.)

Build Matrix โš™๏ธ

This section defines what builds will be run. (NOTE: I actually lifted this from another module, as Elemental 4.3's matrix was in a bit of a weird state.)

matrix:
  include:
    - php: 7.1
      env: DB=PGSQL RECIPE_VERSION=4.3.x-dev PHPCS_TEST=1 PHPUNIT_TEST=1
    - php: 7.2
      env: DB=MYSQL RECIPE_VERSION=4.4.x-dev PHPUNIT_TEST=1
    - php: 7.3
      env: DB=MYSQL RECIPE_VERSION=4.4.x-dev PHPUNIT_COVERAGE_TEST=1
    - php: 7.3
      env: DB=MYSQL RECIPE_VERSION=4.4.x-dev NPM_TEST=1
    - php: 7.3
      env: DB=MYSQL RECIPE_VERSION=4.x-dev PHPUNIT_TEST=1

Each build defines a PHP version to use, and sets environment variables:

  • DB=MYSQL/PGSQL is effectively a boolean that defines whether the PostgreSQL support module will be installed. Some modules don't officially support PostgreSQL, so they lack builds with the PGSQL value.
  • RECIPE_VERSION=4.x-dev defines which version of core will be installed, generally via silverstripe/recipe-core or silverstripe/recipe-cms.

    • At minimum, each of the currently supported core versions and the next version (4.x) should be covered in these builds.

    • We generally prefer development branches to stable tags as they can occasionally catch issues in core before they ship. However, when the development branch of a minor version is removed, we have to fall back to tags if we still want to test them (currently 4.0-4.2 are impacted by this).

  • PHPUNIT_TEST / PHPUNIT_COVERAGE_TEST, PHPCS_TEST, BEHAT_TEST, NPM_TEST: These designate what type of tests will be run. PHPUnit and Behat tests should be run for every PHP and core version, whilst the other test types should only be run once.

The build matrix can't be made identical across all modules or branches, due to a lot of variations:

  • Different minimum core support
  • Different minimum PHP support
  • Some builds install extra modules for cross-module support testing
  • Some modules don't have Behat / JS tests

We can use configuration to generate a matrix based on these variations, with the following tactics:

  • We could reference composer.json for the branch to determine the core / PHP versions that can be tested.
  • For cross-module testing, we could define a custom section in composer.json called require-test, or simply ensure that the extra modules are always installed in the before_script stage via custom config.
  • We'll need to define clear configuration for each module as to whether they have Behat / JS tests to run. It might be possible to infer these based on existing files / config, but I'm not confident this would be reliable.

Regarding --prefer-latest and --prefer-lowest, I think employing the latter to warn us when our core requirements are too low makes sense, but for supported minor core releases we should continue to test each one explicitly to catch UI breakages and usage of APIs that aren't present in prior releases.

Pre-build scripts โœ…

These handle installing dependencies and spinning up required services.

before_script:
  - sudo apt-get remove -y --purge google-chrome-stable || true

  # Init PHP
  - phpenv rehash
  - phpenv config-rm xdebug.ini
  - echo 'memory_limit = 2G' >> ~/.phpenv/versions/$(phpenv version-name)/etc/conf.d/travis.ini

  # Install composer dependencies
  - composer validate
  - composer require silverstripe/recipe-cms:"$RECIPE_VERSION" silverstripe/recipe-testing:^1 --no-update
  - if [[ $BEHAT_TEST ]]; then composer require --no-update silverstripe/behat-extension:4.x-dev; fi;
  - if [[ $DB == PGSQL ]]; then composer require silverstripe/postgresql:^2 --no-update; fi
  - composer install --prefer-source --no-interaction --no-progress --no-suggest --optimize-autoloader --verbose --profile

  # Behat bootstrapping
  - if [[ $BEHAT_TEST ]]; then mkdir artifacts; fi
  - if [[ $BEHAT_TEST ]]; then cp composer.lock artifacts/; fi
  - if [[ $BEHAT_TEST ]]; then (chromedriver > artifacts/chromedriver.log 2>&1 &); fi
  - if [[ $BEHAT_TEST ]]; then (vendor/bin/serve --bootstrap-file vendor/silverstripe/cms/tests/behat/serve-bootstrap.php &> artifacts/serve.log &); fi

There are some common patterns across our modules in before_script, but they're often implemented slightly differently. The core set of steps that have to be run:

  • Purge conflicting Chrome install (a Xenial-specific nuance, this might be removed in the future)
  • Configure PHP (disable XDebug and bump the memory limit)
  • Add Composer requirement for specified core recipe
  • Conditionally add Composer requirements for PostgreSQL and Behat modules
  • Validate / install Composer requirements
  • Bootstrap Behat if required
  • Install NPM dependencies if required

I _think_ we could come up with a standardised config here, but we'll need to expose a method of prepending / appending additional steps for modules that have really unique scenarios. These hooks could be implemented via Composer scripts (e.g. travis-prepend-before-script and travis-append-before-script).

Build scripts โœ…

These are the actions that actually run our tests.

script:
  - if [[ $PHPUNIT_TEST ]]; then vendor/bin/phpunit tests/; fi
  - if [[ $PHPUNIT_COVERAGE_TEST ]]; then phpdbg -qrr vendor/bin/phpunit --coverage-clover=coverage.xml; fi
  - if [[ $PHPCS_TEST ]]; then vendor/bin/phpcs src/ tests/ ; fi
  - if [[ $NPM_TEST ]]; then git diff-files --quiet -w --relative=client; fi
  - if [[ $NPM_TEST ]]; then git diff --name-status --relative=client; fi
  - if [[ $NPM_TEST ]]; then yarn run test; fi
  - if [[ $NPM_TEST ]]; then yarn run lint; fi
  - if [[ $BEHAT_TEST ]]; then vendor/bin/behat @silverstripe-elemental; fi

These are _fairly_ standard already, so I think this could be module-agnostic. The only reference to the specific module in that list is the Behat suite name, which we could potentially shift into a per-module global environment variable (or similarly to the way NPM tests run, we could create a standard Composer script, e.g. composer run-script behat, and have the modules maintain the exact command that is run). We could also offer some hooks for adding custom config if necessary (as with the previous section, my suggestion would be appropriately named Composer scripts)

Post-build Scripts โœ…

These sections cover any required actions based on the outcome of the build.

after_success:
  - if [[ $PHPUNIT_COVERAGE_TEST ]]; then bash <(curl -s https://codecov.io/bash) -f coverage.xml; fi

after_failure:
- php ./vendor/silverstripe/framework/tests/behat/travis-upload-artifacts.php --if-env BEHAT_TEST,ARTIFACTS_BUCKET,ARTIFACTS_KEY,ARTIFACTS_SECRET --target-path $TRAVIS_REPO_SLUG/$TRAVIS_BUILD_ID/$TRAVIS_JOB_ID --artifacts-base-url https://s3.amazonaws.com/$ARTIFACTS_BUCKET/ --artifacts-path ./artifacts/

These aren't present across all modules at the moment (some don't have code coverage, others don't Behat tests) but they're safe enough to standardise. (We can also add some hooks to these steps if necessary.)

Automation

This feels like a job for a bot. We have access to GitHub Actions now, so we can trigger the bot on activities like:

  • Configuration files changing in a module
  • New branches being created
  • Shared Travis configuration changes

We can then feed it a list of modules to manage, some configuration around which core versions are currently supported, and a template to base its output on, and it can start raising PRs whenever changes are required. (We can probably even put it in charge of creating the GitHub Action configuration in each managed repo ๐Ÿ˜„)

The RFC

Per the official process, we need to get this out for a core committer vote in order to move forward with an implementation. @chillu, do you feel there's a sufficient amount of information present to go forward with a proof of concept?

Sounds amazing, nice work on the investigation!

Great work Garion! Sorry it took me so long to get around to this.

Regarding --prefer-latest and --prefer-lowest, I think employing the latter to warn us when our core requirements are too low makes sense, but for supported minor core releases we should continue to test each one explicitly to catch UI breakages and usage of APIs that aren't present in prior releases.

So there's two different costs associated to this:

  • Cost of developing automation (one off). Roughly same if we only automate a few parts (e.g. just PHP) or all of it (e.g. PHP, core version, other config settings)
  • Cost of merging pull requests created by automation (ongoing). Increases with the amount of pull requests we need to merge (e.g. just PHP or on every core version EOL). Factoring in that we need to wait for each of those builds to turn green before merge.

The way I understand it, the automation you propose will create at least ~100 pull requests every few months as our 4.x minor releases become EOL and we need to increase minimum core requirements. Is that a worthwhile tradeoff for more certainty regarding potential API breakages based on older minor releases? Those breakages would constitute a failure of our documented process around which branch to target with API changes, but definitely good to have a safeguard.

Branch Target Option A (Garion): Test 4.3 and 4.4 (while 4.3 and 4.4 are supported). Once 4.5 comes out, test 4.3 and 4.5. Once 4.3 goes EOL, test 4.4 and 4.5.

Branch Target Option B (Ingo): Test 4.0 and 4.4 (while 4.3 and 4.4 are supported). Once 4.5 comes out, test 4.0 and 4.5. Once 4.3 goes EOL, nothing changes.

After all, 4.0 is the dependency constraint we set in those modules. Where 4.0 tests fail based on reasons we can't bother to debug, the default reaction should be to bump the required core dependency for the module to the lowest supported one. It might be worth doing that proactively for all supported modules as a one off, on the assumption that it's unlikely we need to bump the lowest supported core version again soon due to test failures (so it'd stay 4.3 for a while, even past its EOL date).

So Option B reduces the need for automation, but doesn't remove it. More importantly, it reduces the ongoing maintenance need in merging automated pull requests. I'm not sure that's a worthwhile tradeoff, but let's be clear that the automation you're proposing would sign us up for merging shitloads of pull requests on a continued basis ;)

However, when the development branch of a minor version is removed, we have to fall back to tags if we still want to test them

Didn't we decide that we keep those branches now? Can't find a ticket reference.

Didn't we decide that we keep those branches now? Can't find a ticket reference.

Yes, going forwards this isn't an issue - and I think we've successfully cleared the 4.1 / 4.2 branches out of most modules (either replacing them with tags, or removing them entirely).

Those breakages would constitute a failure of our documented process around which branch to target with API changes, but definitely good to have a safeguard.

Not necessarily - consider that we have test coverage of our JS components, which we've explicitly documented as "could break in minors". There's also API _additions_, which are perfectly allowed in minor releases and could be used without awareness.

let's be clear that the automation you're proposing would sign us up for merging shitloads of pull requests on a continued basis ;)

What I'm signing us up for is merging shitloads of pull requests, as a _reduction in effort_ from _raising_ shitloads of pull requests by hand _and_ merging them. This is definitely still a lot of work, so I can see the value in removing specific version references entirely. I'm pretty torn between the options at this point.

Here's a bold question: Is it worth considering automated merges if these automated PRs pass CI? ๐Ÿค”

Is it worth considering automated merges if these automated PRs pass CI?

Given we often skip PRs for travis config _only_ updates on "satellite" repos, imo yes

After all, 4.0 is the dependency constraint we set in those modules.

This isn't correct for all our modules. Many minor releases will target a >4.0 release of a core library. Biggest candidate for this is silverstripe/admin.

I think I'm a little confused about the quote in your reply @chillu but for the record; I'm an advocate of using --prefer-lowest on one build in the CI setup, but the goal of this should be to verify the constraints set in the module are correct. It would be an uncommon side-effect that the builds break because of an API change in the dependency.

Haven't digested your comments yet, went down a rabbit hole instead :D This would be a good opportunity to make an explicit decision about continuing to use Travis over alternatives like Github Actions and CircleCI, because we'd be investing further into Travis-specific tooling here. I've started a discussion at https://forum.silverstripe.org/t/should-we-use-github-actions-for-supported-module-builds/2514

Travis has a new beta feature for shared build configs! https://docs.travis-ci.com/user/build-config-imports/

@silverstripe/core-team hi team!

Here's a quick update regarding the new TravisCI feature (reusable configs).

@emteknetnz has performed some initial research (in a Hackday), then we tried it on a real module to check out how it works.

The initial plan was to play with it on a couple of satellite modules, stabilise, and start gradually rolling out to some more, solving the issues along the way.
However, the result was so good, that it is actually much easier for us to swap the existing configs to it rather than patch every existing module own .travis.yml. This can help us spare very good amount of effort and time before the upcoming release (4.7/2.7) and we have a good amount of broken builds out there (about 80ish in total), so we've decided to give it a go for all the rest of the satellite modules.

Here is the repository with the reusable configs: https://github.com/silverstripe/silverstripe-travis-shared

We've only been working on it for several days so far, so it is still a work in progress. Nonetheless, we already had a good amount of discussion within the CMS Squad team and keen to hear any feedback or concerns regarding the approach from the Core Committers team and the community members. Also, any notes on the particular configs (e.g. how the yaml configs should be) can be discussed in the issues of the silverstripe-travis-shared repository.

The work is still in progress and we're planning to roll it out for more modules eventually, but here are some examples of the updates we're working on currently.

Very promising! Looking at https://github.com/silverstripe/silverstripe-travis-shared/, I find it quite hard to follow what actually runs, I've counted up to five levels of include nesting before I get to an actual command being run. Partial snapshot of the "simplest" example (standard-jobs-range.yml) below.

config/provision/standard-jobs-range.yml
  config/provision/standard.yml
    config/provision/base.yml
      config/env/xenial.yml
      config/before_script/main.yml
        config/before_script/npm.yml
          - if [[ $NPM_TEST ]]; then rm -rf client/dist && nvm install && nvm use && npm install -g yarn && yarn install --network-concurrency 1 && (cd vendor/silverstripe/admin && yarn install --network-concurrency 1) && yarn run build; fi
        config/before_script/behat.yml
        config/before_script/composer.yml
        config/before_script/phpenv.yml
      config/script/main.yml
        config/script/phpunit.yml
        config/script/behat.yml
        config/script/phpcs.yml
        config/script/npm.yml
  config/jobs/installer-range.yml
  ...

This might eventually be solved with the Travis Config Explorer flattening the config, but it doesn't seem to support imports at the moment. Is this level of depth really necessary? It's not a blocker, in the end the CMS Squad is doing 90% of the build maintenance work. But I find it hard to see this being adopted by others, or understood by maintainers outside of this team. Maybe you could flatten a few layers with a bit more duplication? Or spell out the purpose of each folder/layer in more in depth examples in the README?

Have you discussed how a change to these includes could be propagated? For example, if you're preparing a 4.8 release by creating release branches, the last build on a given module (e.g. blog) would've run against 4.x-dev and 4.7. You can add 4.8 to the build matrix via the shared config, but that won't trigger a build on blog. So you might accidentally break blog through commits to the 4.8 core modules and not find out about it until an unrelated commit to blog, potentially after the 4.8.0 release.

Scheduled builds on modules would avoid this, but also increase our build resource needs. Maybe we could script this to re-run the last build on the latest release branch of each supported module? Again, not a blocker for this effort, we've already had this problem before shared travis configs by simply not adding new release branches to each .travis.yml.

I find it quite hard to follow what actually runs

Yes, fair point. The current approach is rather somewhat Work In Progress and it allows us to identify all the most granular pieces of configs that are reusable across modules and different build matrices. When we find the setup that suits all modules, we should see much cleaner what are the common patterns there and then we'll be able to refactor that, flatten it better and write more documentation. Right now the main goal is to make it work and have it reusable with the fewest possible amount of customisations on every module level (in the modules' own .travis.yml configs).

Have you discussed how a change to these includes could be propagated?

No, we didn't explicitly discuss this yet. However, I assume it may be easy enough to trigger necessary builds via Travis API. It has a feature "Trigger build" for any branch without modifications for source repositories.

Scheduled builds on modules would avoid this, but also increase our build resource needs

That would be my favoured approach, something like re-running modules on a weekly basis would avoid the situation we were in where we hadn't run builds for upwards of 2 years.

If the builds were staggered over the week and/or done overnight NZ time, then this should have a minimal impact on the availability of travis workers during business hours.

This approach would pretty much exclude being on a travis plan where we purchase a number of hours and allow us to have higher concurrency, rather that having 5 workers with unlimited hours, however I would favour this as we are optimising for stability over outright performance. Also 5 workers seems to be functioning pretty well right now.

I think having a lag of a week before we find out about any issues (potentially more depending on how disciplined we are looking at the travis builds dashboard) is acceptable since our release cadence is only quarterly at this stage.

The travis recurring builds are somewhat simplistic. The kind of automated build schedule you are talking about would probably require some sort of side process to trigger the builds via the API ... which probably would be relatively easy to achieve.

I'm thinking maybe some sort of lambda function running on a schedule or some similar mechanic.

Yes something like a lambda would be ideal so that we can properly control timing. I noticed that a bunch of cron builds triggered around 1pm Thursday NZ time and clogged things up which is certainly less than ideal.

My single concern with this approach would be how we go about key/token management

I'd prefer a scheduled Github action over a Lambda because of secret management, and keeping those concerns in the same place. You can use encrypted secrets there. That's not blocking the conclusion of this RFC though, just a bit of follow up refinement (since the lack of scheduled runs is already the status quo)

Since the work on migrating to shared config in Travis CI is well underway, I'm marking this RFC as accepted. There are still some important follow-on questions being posed here, so I don't think we'll want to close this issue until we've either addressed those items or created separate issues to cover them.

It appears that the only follow up work would be around scheduled builds. Technically that's fixing a regression introduced by this shared Travis config, but in practice it's an existing issue because we simply don't update .travis.yml consistently. So I'd be fine leaving that battle for another day. Especially since we've got a dashboard which highlights "expired" builds already (last build more than 30 days ago) in https://maxime-rainville.github.io/travis-dashboard/. And it's a non-issue for active branches on modules, which are the ones we care about the most. Given the extreme amount of time we spend here on build and release tooling, I think that can go on the wishlist, it's not a blocker. Worst case, once we roll out a an important new build variation (e.g. PHP 8.1 support), we click "rebuild" on a bunch of important modules, and leave the "long tail" of less active modules alone.

That's a nice looking dashboard!

@steve-silverstripe Is this done now?

Yes - https://github.com/silverstripe/silverstripe-travis-shared is now used on virtually all our modules

It appears that the only follow up work would be around scheduled builds.

I believe @emteknetnz manually went through each build on Travis and set up reoccurring build crons for the current major branch (i.e. 4) and perhaps more? That helps with modules that we don't touch often but may have test failures due to integration issues.

Was this page helpful?
0 / 5 - 0 ratings