Skip to content

Commit

Permalink
workflows/tests: enable more granular control of deps jobs
Browse files Browse the repository at this point in the history
Currently, we request long timeouts for PRs whenever at least one of the
formula(e) tests or dependent tests require a long timeout.

This is wasteful of CI resources, because a typical PR that requires a
long timeout only requires it for only one of the formula(e) tests or
the dependent tests.

To avoid unnecessarily congesting the long build queue, let's allow more
granular control of which jobs get queued for a long build by using a
separate label for dependent testing jobs that require a long timeout.

I've also removed the hard-coded values for the short and long timeouts
from the `check-labels.js` script so that we can keep these in `brew`
instead (and now we'll only need to keep track of them in one place
instead of in two).
  • Loading branch information
carlocab committed May 11, 2024
1 parent 273b263 commit fb0a643
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 21 deletions.
42 changes: 27 additions & 15 deletions .github/workflows/scripts/check-labels.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
module.exports = async ({github, context, core}, formulae_detect) => {
module.exports = async ({github, context, core}, formulae_detect, dependent_testing) => {
const deps_suffix = dependent_testing ? '-deps' : ''

const { data: { labels: labels } } = await github.rest.pulls.get({
owner: context.repo.owner,
repo: context.repo.repo,
Expand All @@ -23,18 +25,18 @@ module.exports = async ({github, context, core}, formulae_detect) => {
}

var linux_runner = 'ubuntu-22.04'
if (label_names.includes('CI-linux-self-hosted')) {
if (label_names.includes(`CI-linux-self-hosted${deps_suffix}`)) {
linux_runner = 'linux-self-hosted-1'
} else if (label_names.includes('CI-linux-large-runner')) {
} else if (label_names.includes(`CI-linux-large-runner${deps_suffix}`)) {
linux_runner = 'homebrew-large-bottle-build'
}
core.setOutput('linux-runner', linux_runner)

if (label_names.includes('CI-no-fail-fast')) {
console.log('CI-no-fail-fast label found. Continuing tests despite failing matrix builds.')
if (label_names.includes(`CI-no-fail-fast${deps_suffix}`)) {
console.log(`CI-no-fail-fast${deps_suffix} label found. Continuing tests despite failing matrix builds.`)
core.setOutput('fail-fast', false)
} else {
console.log('No CI-no-fail-fast label found. Stopping tests on first failing matrix build.')
console.log(`No CI-no-fail-fast${deps_suffix} label found. Stopping tests on first failing matrix build.`)
core.setOutput('fail-fast', true)
}

Expand All @@ -49,12 +51,22 @@ module.exports = async ({github, context, core}, formulae_detect) => {
core.setOutput('test-dependents', true)
}

if (label_names.includes('long build')) {
console.log('"long build" label found. Setting long GitHub Actions timeout.')
core.setOutput('timeout-minutes', 4320)
if (dependent_testing) {
if (label_names.includes('long dependent tests')) {
console.log('"long dependent tests" label found. Setting long GitHub Actions timeout.')
core.setOutput('long-timeout', true)
} else {
console.log('No "long dependent tests" label found. Setting short GitHub Actions timeout.')
core.setOutput('long-timeout', false)
}
} else {
console.log('No "long build" label found. Setting short GitHub Actions timeout.')
core.setOutput('timeout-minutes', 120)
if (label_names.includes('long build')) {
console.log('"long build" label found. Setting long GitHub Actions timeout.')
core.setOutput('long-timeout', true)
} else {
console.log('No "long build" label found. Setting short GitHub Actions timeout.')
core.setOutput('long-timeout', false)
}
}

const test_bot_formulae_args = ["--only-formulae", "--junit", "--only-json-tab", "--skip-dependents"]
Expand All @@ -65,12 +77,12 @@ module.exports = async ({github, context, core}, formulae_detect) => {
const test_bot_dependents_args = ["--only-formulae-dependents", "--junit"]
test_bot_dependents_args.push(`--testing-formulae="${formulae_detect.testing_formulae}"`)

if (label_names.includes('CI-test-bot-fail-fast')) {
console.log('CI-test-bot-fail-fast label found. Passing --fail-fast to brew test-bot.')
if (label_names.includes(`CI-test-bot-fail-fast${deps_suffix}`)) {
console.log(`CI-test-bot-fail-fast${deps_suffix} label found. Passing --fail-fast to brew test-bot.`)
test_bot_formulae_args.push('--fail-fast')
test_bot_dependents_args.push('--fail-fast')
} else {
console.log('No CI-test-bot-fail-fast label found. Not passing --fail-fast to brew test-bot.')
console.log(`No CI-test-bot-fail-fast${deps_suffix} label found. Not passing --fail-fast to brew test-bot.`)
}

if (label_names.includes('CI-build-dependents-from-source')) {
Expand Down Expand Up @@ -123,7 +135,7 @@ module.exports = async ({github, context, core}, formulae_detect) => {
}

if (label_names.includes('CI-skip-new-formulae-strict')) {
console.log('CI-skip-new-formulae-strict label found. Passing --skip-new-strictw to brew test-bot.')
console.log('CI-skip-new-formulae-strict label found. Passing --skip-new-strict to brew test-bot.')
test_bot_formulae_args.push('--skip-new-strict')
} else {
console.log('No CI-skip-new-formulae-strict label found. Not passing --skip-new-strict to brew test-bot.')
Expand Down
12 changes: 6 additions & 6 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ jobs:
linux-runner: ${{ steps.check-labels.outputs.linux-runner }}
fail-fast: ${{ steps.check-labels.outputs.fail-fast }}
test-dependents: ${{ steps.check-labels.outputs.test-dependents }}
timeout-minutes: ${{ steps.check-labels.outputs.timeout-minutes }}
long-timeout: ${{ steps.check-labels.outputs.long-timeout }}
test-bot-formulae-args: ${{ steps.check-labels.outputs.test-bot-formulae-args }}
test-bot-dependents-args: ${{ steps.check-labels.outputs.test-bot-dependents-args }}
steps:
Expand All @@ -126,7 +126,7 @@ jobs:
}
try {
await script({github, context, core}, formulae_detect)
await script({github, context, core}, formulae_detect, false)
} catch (error) {
console.error(error);
}
Expand All @@ -144,7 +144,7 @@ jobs:
runners_present: ${{steps.determine-runners.outputs.runners_present}}
env:
HOMEBREW_LINUX_RUNNER: ${{needs.setup_tests.outputs.linux-runner}}
HOMEBREW_MACOS_TIMEOUT: ${{needs.setup_tests.outputs.timeout-minutes}}
HOMEBREW_MACOS_LONG_TIMEOUT: ${{needs.setup_tests.outputs.long-timeout}}
TESTING_FORMULAE: ${{needs.formulae_detect.outputs.testing_formulae}}
DELETED_FORMULAE: ${{needs.formulae_detect.outputs.deleted_formulae}}
steps:
Expand Down Expand Up @@ -264,7 +264,7 @@ jobs:
linux-runner: ${{ steps.check-labels.outputs.linux-runner }}
fail-fast: ${{ steps.check-labels.outputs.fail-fast }}
test-dependents: ${{ steps.check-labels.outputs.test-dependents }}
timeout-minutes: ${{ steps.check-labels.outputs.timeout-minutes }}
long-timeout: ${{ steps.check-labels.outputs.long-timeout }}
test-bot-formulae-args: ${{ steps.check-labels.outputs.test-bot-formulae-args }}
test-bot-dependents-args: ${{ steps.check-labels.outputs.test-bot-dependents-args }}
steps:
Expand All @@ -291,7 +291,7 @@ jobs:
}
try {
await script({github, context, core}, formulae_detect)
await script({github, context, core}, formulae_detect, true)
} catch (error) {
console.error(error);
}
Expand All @@ -310,7 +310,7 @@ jobs:
runners_present: ${{steps.determine-dependent-runners.outputs.runners_present}}
env:
HOMEBREW_LINUX_RUNNER: ${{needs.setup_dep_tests.outputs.linux-runner}}
HOMEBREW_MACOS_TIMEOUT: ${{needs.setup_dep_tests.outputs.timeout-minutes}}
HOMEBREW_MACOS_LONG_TIMEOUT: ${{needs.setup_dep_tests.outputs.long-timeout}}
TESTING_FORMULAE: ${{needs.formulae_detect.outputs.testing_formulae}}
steps:
- name: Set up Homebrew
Expand Down

0 comments on commit fb0a643

Please sign in to comment.