Skip to content

Build each pull request on the configurations its changes need - #43

Merged
jwrosewell merged 8 commits into
rcfrom
pull-request-depth
Nov 14, 2026
Merged

jwrosewell merged 8 commits into
rcfrom
pull-request-depth

Conversation

@jwrosewell

@jwrosewell jwrosewell commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Problem

A pull request into rc runs all 15 configurations in ci/options.json, whatever it changes. Six of them are macOS, and only five macOS jobs run at once across the whole organisation. Every configuration runs the three IP Intelligence tests that load the multi-gigabyte Enterprise data file, every Windows configuration installs WMIC, which is only there for one of those tests, and fourteen configurations run the Selenium contract, the flakiest check in the window.

Cost, measured from run history

  • The one build since 2 November where every job passed first time (run 41531885450, #22) waited 109 minutes from the first job queued to the last finished and used 601 runner minutes.
  • The change to comments, a message and a link in #32 (run 41708521029) took 143 minutes and 1,006 runner minutes over three attempts, because a Mockito failure on Ubuntu Java 17 and a Selenium timeout on macOS Intel Java 17 each cancelled the rest.
  • In Ubuntu Java 17, 25 minutes, IPIntelligenceTests, MetadataOnPremTest and CompareOnPremTest took 12.5 of the 16.7 unit test minutes. In Windows Java 17, 56 minutes, WMIC took 12.5 minutes to install and the same three tests 22.7.
  • Four by-hand runs on 2 November found no eligible pull request and built rc in full, using 1,234 runner minutes. Configurations cancelled when another failed threw away 1,148 runner minutes in the window.
  • On 4 November the nightly (run 41790027166) built pull request Update submodules #38 on all 15 configurations, starting its six macOS jobs between 07:29 and 08:10, before the publish began at 08:59. The publish's macOS jobs then waited 18 to 54 minutes for a runner.

Change

  1. A new ci/select-options.ps1 chooses the configurations for common-ci's selection step, which common-ci#309 added.
    • Standard set, for every change. Ubuntu_Java_17 and Ubuntu_Java_8, the main JDK and the oldest, with the Selenium contract on Ubuntu_Java_17 alone.
    • A change to a native module or the cxx submodule, in more than its comments, adds Windows_Java_17 and Ubuntu_ARM_Java_17, where the native library is built differently.
    • A pom.xml change adds Ubuntu_Java_11 and Ubuntu_Java_21.
    • A change outside the package trees, such as ci, adds every configuration but macOS.
    • Only when rc itself or a release is built do the six macOS configurations and the performance tests run.
  2. At the standard depth, ci/run-unit-tests.ps1 leaves out the three tests in the new ci/standard-excludes.txt through Surefire's surefire.excludesFile, and ci/setup-environment.ps1 does not install WMIC. A change to IP Intelligence or the cxx submodule runs them.
  3. ci/run-integration-tests.ps1 runs the Selenium contract only where the selection leaves SeleniumContract true, which it is by default.
  4. The Pull Requests workflow passes fail-fast: false and build-without-pull-requests: false, and gains a full-matrix box for a full build of a pull request by hand.
  5. The nightly runs its publish first and builds pull requests after it, as the publish builds main, which the pull requests into rc do not change.
  6. A new release-pull-request.yml builds the release pull request from rc into main on every configuration, as a dry run that merges nothing.
  7. ci/README.md describes the selection.
  8. ci/select-options.Tests.ps1 holds seven Pester 5 tests of the selection, one for each kind of change (below).
  9. The "Cloud 51Did Tests" workflow runs on pull requests into rc as well as main. Changes reach main only through rc, so a pull request that changed the 51Did package had not run them since the move to rc.
  10. A new .github/workflows/ci-script-tests.yml runs every Pester test in ci on ubuntu-latest and windows-latest, for each pull request that changes ci or the workflow and each push of such a change to main or rc, so the tests in item 8 keep running once this merges. It leaves out ci/check-environment-variable-names.Tests.ps1, which the Environment variable names workflow already runs. ci/README.md names it beside the tests.

Expected saving

From the job times above, by the kind of change.

Change Configurations Wait Runner minutes
Today, any change 15 109 601
Documentation alone 0 0 0
Code or comments in a package tree, outside IP Intelligence 2 about 12 about 20
IP Intelligence 2 about 24 about 45
A pom.xml 4 about 12 about 43
A native module or the cxx submodule 4 about 55, set by Windows about 120
CI or other files outside the package trees 9 about 65 about 358

The reduced times take the measured minutes of the three tests, WMIC and performance off the median of each configuration. Applied to the changed files of the 26 pull requests opened since 1 November, many of them CI set-up, the median wait falls from 109 to about 40 minutes and the mean runner minutes from 601 to about 151. Over 2 and 3 November the change, the fail-fast setting and the by-hand builds of rc it stops come to about 2,300 runner minutes a day. The release pull request adds one full build, about 601 runner minutes, for each release, and no pull request build takes a macOS runner.

How it was checked

  • Every changed PowerShell file parses, and every changed workflow parses as YAML. The jobs moved in nightly-pipeline.yml are identical to the old ones apart from their order, needs, if and the new input.
  • The class names and packages in ci/standard-excludes.txt are the ones the Ubuntu Java 17 log reports, and the Device Detection MetadataOnPremTest is not matched. Surefire 2.22.2, which the poms name, reads the file from the surefire.excludesFile property and ignores its # lines (SurefirePlugin.java). A file of excludes replaces Surefire's own **/*$* default (AbstractSurefireMojo.java), so the file lists it too.
  • The selection rules were applied, in a copy, to the changed files of every pull request since 1 November, which gives the table above.
  • common-ci main has run a repository's ci/select-options.ps1 for a pull request since common-ci#312 on 12 November, unless the build is of the branch itself or asks for the full matrix (nightly-pull-request.configure-pr.ps1 lines 36 to 42). rc has no ci/select-options.ps1, so today every pull request into rc builds every configuration, and once this merges the nightly's pull request builds choose as above.
  • common-ci's own step against this script. A temporary workflow (run 43529672039, on 8baec75) checked out common-ci main at the root, made one local commit for each kind of change on this branch, and ran common-ci's steps/select-options.ps1 on it as the nightly does. A README alone chose nothing, a change to pipeline.core the standard set, a comment in a native module the standard set, a change to IP Intelligence the standard set at depth deep, a pom.xml change JDK 11 and 21 as well, and a ci script every entry but macOS at depth deep. A change to a native module, and a new commit of the cxx submodule, chose all nine entries but macOS where four were meant. PowerShell names are not case sensitive, so $native, set to whether a native module changed, had replaced the list $Native of the entries a native change adds, and $true -contains '<name>' is true for every name.
  • The fix, tests first. 2c8c4606 adds ci/select-options.Tests.ps1, and 7615d50c names the list $NativeEntries. Red, run 43531443600 on tmp/java-43-red (2c8c460 with a temporary workflow that runs the file with Invoke-Pester on ubuntu-latest and windows-latest), failed the native module and cxx tests, each choosing the nine, and passed the other five, on both systems. Green, run 43531443597 on tmp/java-43-green (7615d50), passed all seven on both.
  • The tests on every pull request, from the review of 04:03 UTC on 14 November, which found that no workflow ran ci/select-options.Tests.ps1. 32095737 adds ci-script-tests.yml (item 10), and 4aa6c312 names it in ci/README.md. Red, run 43558755258 on the throwaway branch tmp/java-43-wf-red (3209573 with ci/select-options.ps1 as it was at 2c8c460, before its fix, and the workflow set to run on that branch), failed the native module and cxx tests and passed the other five, on both systems. Green, the workflow's own run on this head, run 43559030818, ran select-options.Tests.ps1 and passed all seven on both systems.
  • CI scripts and workflows only, so under James Rosewell's decision of 11:35 UTC on 12 November ("rebuild and include, but the change doesn't impact the code so no need to run the full builds") this merges on green quick checks and a review, with no full build of its own. The checks on this head, 4aa6c31, are green, being Only rc into main, Environment variable names, UTM link lint and CI script tests. Because this branch changes the 51Did workflow, that workflow ran on this pull request into rc, run 43559030813, and passed.

What it gives up

On a pull request, macOS, JDK 11 and 21 unless a pom.xml changes, the Windows and ARM native builds unless native code changes, the three IP Intelligence tests unless IP Intelligence changes, thirteen of the fourteen Selenium runs, and performance. A fault that shows only there is found by the nightly build of rc or by the release pull request build, before anything reaches main.

Merge order

The common-ci half, common-ci#309, reached common-ci main in common-ci#312 on 12 November, so the inputs these workflows pass are there. This merges into rc last of the drafts for the 16 November release. Once it is in rc, a full build of a later draft started by hand chooses fewer configurations unless it passes full-matrix: true.

Merges with rc

rc was merged in at 8baec75, with a merge commit, bringing 96 commits. Three files conflicted.

  • ci/run-unit-tests.ps1. rc now calls common-ci's java/run-unit-tests.ps1 with the Maven arguments in ExtraArgs. The merge keeps that call and adds this branch's surefire.excludesFile argument to ExtraArgs when the depth is standard, with the Depth parameter kept.
  • ci/run-integration-tests.ps1. The merge keeps this branch's SeleniumContract switch and rc's wording, as the contract now runs against two web examples.
  • ci/README.md. The merge keeps this branch's paragraph on select-options.ps1 and rc's on the asset cache.

rc was merged in again at f099547, bringing #127, #128 and #130, with no conflict.

The three tests in ci/standard-excludes.txt still exist under the same names on rc.

What remains

  • The cause of the Mockito failure in DataUpdateServiceTests.DataUpdateService_Register_AutoUpdateConfiguredInterval and of the Selenium WebDriver timeouts, neither established.
  • The nightly still starts at 03:00. The Node and Python nightlies move away from that time in their own pull requests.

A pull request into rc runs all 15 configurations whatever it changes,
six of them on macOS, of which only five run at once across the
organisation. The one build where every job passed first time waited
109 minutes and used 601 runner minutes. A change to comments, a message
and a link took 143 minutes and 1,006 runner minutes over three attempts.

- ci/select-options.ps1 chooses the configurations for common-ci. The
  standard set is JDK 17 and JDK 8 on Linux. A change to a native module
  or the cxx submodule adds Windows and Linux ARM, a pom.xml change adds
  JDK 11 and 21, and a change outside the package trees, such as ci,
  adds every configuration but macOS. macOS and performance run only
  when rc itself or a release is built.
- The standard set leaves out the three IP Intelligence tests that load
  the Enterprise data file, listed in ci/standard-excludes.txt, and so
  does not install WMIC, which is only there for them. A change to IP
  Intelligence or the cxx submodule runs them.
- The Selenium contract runs on one configuration of a pull request
  rather than fourteen.
- The Pull Requests workflow lets one failure leave the other
  configurations running, and builds nothing when no pull request is
  eligible. The nightly publishes before it builds pull requests, as the
  publish builds main, so it has the runners first.
- The release pull request from rc into main is built on every
  configuration.

These need common-ci's fail-fast, full-matrix and
build-without-pull-requests inputs and its release-pull-request.yml on
common-ci main before this reaches main.
PowerShell names are not case sensitive, so $native, set to whether a
native module changed, replaced the list $Native of the entries a native
change adds. $true -contains '<name>' is true for every name, so a change
to a native module or the cxx submodule built every entry but macOS.
Changes reach main only through rc, so a pull request that changes the
51Did package had not run these tests since the move to rc.
@jwrosewell
jwrosewell marked this pull request as ready for review November 14, 2026 06:04
@jwrosewell
jwrosewell merged commit ceab088 into rc Nov 14, 2026
6 checks passed
@jwrosewell
jwrosewell deleted the pull-request-depth branch November 14, 2026 06:04
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

Successfully merging this pull request may close these issues.

1 participant