Skip to content

Commit

Permalink
Cleanup infra issues (#736)
Browse files Browse the repository at this point in the history
- `remark-lint`'s `extra_packages` released new changes, so I've
refreshed the snapshots. We don't pin versions unless explicit, so this
causes some mild hermeticity issues (but is still an acceptable edge
case)
- Change to use `ubuntu-latest` runners until we find a more sustainable
solution for this repo (see nightly failures)
- Mild workflow cleanup

Takes over #725 and addresses [nightly
issues](https://github.com/trunk-io/plugins/actions/runs/8612368048)
  • Loading branch information
TylerJang27 authored Apr 10, 2024
1 parent 40d07cf commit 5611fee
Show file tree
Hide file tree
Showing 24 changed files with 84 additions and 37 deletions.
1 change: 1 addition & 0 deletions .github/actionlint.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ self-hosted-runner:
labels:
- ubuntu-x64
- macOS
- ubuntu-latest
# Configuration variables in array of strings defined in your repository or
# organization. `null` means disabling configuration variables check.
# Empty array means no configuration variable is allowed.
Expand Down
4 changes: 2 additions & 2 deletions .github/actions/linter_tests/action.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ runs:
# Install non-hermetic linters
sudo apt-get update
sudo apt-get -y install libperl-critic-perl perltidy zlib1g-dev
sudo apt-get -y install libperl-critic-perl perltidy zlib1g-dev software-properties-common
DEBIAN_FRONTEND=noninteractive TZ=Etc/UTC sudo apt-get -y install tzdata
sudo add-apt-repository ppa:ondrej/php
sudo apt install -y php8.0-fpm php8.0-xml php8.0-mbstring php8.0-curl
Expand Down Expand Up @@ -98,7 +98,7 @@ runs:
working-directory: ${{ inputs.path }}
trunk-token: ${{ inputs.trunk-token }}
org: trunk-staging-org
run: npm test ${{ inputs.append-args }} ${{ env.PLATFORM_APPEND_ARGS }} --ci
run: npm test ${{ inputs.append-args }} ${{ env.PLATFORM_APPEND_ARGS }} --ci --runInBand
env:
PLUGINS_TEST_LINTER_VERSION: ${{ inputs.linter-version }}
PLUGINS_TEST_CLI_VERSION: ${{ inputs.cli-version }}
Expand Down
23 changes: 13 additions & 10 deletions .github/workflows/nightly.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ jobs:
fail-fast: false
matrix:
linter-version: [Snapshots, Latest]
os: [ubuntu-x64, macOS, windows-latest]
os: [ubuntu-latest, macOS, windows-latest]
steps:
- name: Checkout
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
Expand Down Expand Up @@ -80,10 +80,10 @@ jobs:
fail-fast: false
matrix:
linter-version: [Snapshots, Latest]
os: [ubuntu-x64, macOS, windows-latest]
os: [ubuntu-latest, macOS, windows-latest]
include:
# Normalize the filenames as inputs for ease of parsing
- os: ubuntu-x64
- os: ubuntu-latest
results-file: ubuntu-latest
- os: macOS
results-file: macos-latest
Expand Down Expand Up @@ -157,17 +157,20 @@ jobs:

- name: Delete cache
# For now, avoid deleting cache on pull request changes to nightly. This improves PR experience.
if: env.TRIGGER != 'pull_request'
run: |
if [ -d "/tmp/plugins_testing_download_cache" ]
if [ -d "${TMPDIR:-/tmp}/plugins_testing_download_cache" ]
then
tmp_dir=/tmp/${GITHUB_RUN_ID}-${GITHUB_RUN_NUMBER}-${GITHUB_RUN_ATTEMPT}
mv "/tmp/plugins_testing_download_cache" ${tmp_dir}
tmp_dir=${TMPDIR:-/tmp}/${GITHUB_RUN_ID}-${GITHUB_RUN_NUMBER}-${GITHUB_RUN_ATTEMPT}
mv "${TMPDIR:-/tmp}/plugins_testing_download_cache" ${tmp_dir}
chmod -R u+w ${tmp_dir}
rm -rf ${tmp_dir}
fi
shell: bash

- name: Clean jest cache
run: npx jest --clearCache
shell: bash

- name: Linter Tests ${{ matrix.os }}
# Use overwritten dependency action, rather than released version
uses: ./.github/actions/linter_tests
Expand Down Expand Up @@ -215,10 +218,10 @@ jobs:
strategy:
fail-fast: false
matrix:
os: [ubuntu-x64, macOS, windows-latest]
os: [ubuntu-latest, macOS, windows-latest]
include:
# Normalize the filenames as inputs for ease of parsing
- os: ubuntu-x64
- os: ubuntu-latest
results-file: ubuntu-latest
- os: macOS
results-file: macos-latest
Expand Down Expand Up @@ -268,7 +271,7 @@ jobs:

action_tests_main:
name: Action Tests Main
runs-on: [self-hosted, Linux]
runs-on: [ubuntu-latest]
timeout-minutes: 30
steps:
- name: Checkout
Expand Down
24 changes: 19 additions & 5 deletions .github/workflows/pr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ jobs:
# Run tests against all linters for known_good_version and latest version
linter_tests:
name: Linter Tests ${{ matrix.os }}
runs-on: [self-hosted, "${{ matrix.os }}"]
runs-on: ${{ matrix.os }}
needs: detect_changes
if:
needs.detect_changes.outputs.linters == 'true' || needs.detect_changes.outputs.all-linters ==
Expand All @@ -126,11 +126,25 @@ jobs:
strategy:
fail-fast: false
matrix:
os: [Linux, macOS]
os: [ubuntu-latest, macOS]
steps:
- name: Checkout
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1

# TODO(Tyler): Remove this once the cache has stabilized
- name: Delete cache (mac only)
if: matrix.os == 'macOS'
# For now, avoid deleting cache on pull request changes to nightly. This improves PR experience.
run: |
if [ -d "${TMPDIR:-/tmp}/plugins_testing_download_cache" ]
then
tmp_dir=${TMPDIR:-/tmp}/${GITHUB_RUN_ID}-${GITHUB_RUN_NUMBER}-${GITHUB_RUN_ATTEMPT}
mv "${TMPDIR:-/tmp}/plugins_testing_download_cache" ${tmp_dir}
chmod -R u+w ${tmp_dir}
rm -rf ${tmp_dir}
fi
shell: bash

- name: Linter Tests
# Run tests using KnownGoodVersion with any modified linters and conditionally all linters
uses: ./.github/actions/linter_tests
Expand All @@ -156,7 +170,7 @@ jobs:

tool_tests:
name: Tool Tests
runs-on: [self-hosted, "${{ matrix.os }}"]
runs-on: ${{ matrix.os }}
needs: detect_changes
if:
needs.detect_changes.outputs.tools == 'true' || needs.detect_changes.outputs.all-tools ==
Expand All @@ -165,7 +179,7 @@ jobs:
strategy:
fail-fast: false
matrix:
os: [Linux, macOS]
os: [ubuntu-latest, macOS]
steps:
- name: Checkout
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
Expand All @@ -182,7 +196,7 @@ jobs:

action_tests:
name: Action Tests
runs-on: [self-hosted, Linux]
runs-on: ubuntu-latest
needs: detect_changes
if:
needs.detect_changes.outputs.actions == 'true' || needs.detect_changes.outputs.all-actions ==
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/repo_tests.reusable.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ permissions: read-all
jobs:
plugins_test:
name: Plugin Tests
runs-on: ubuntu-x64
runs-on: ubuntu-latest
timeout-minutes: 15

steps:
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/upload_results.reusable.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ permissions: read-all
jobs:
upload_test_results:
name: Upload Test Results
runs-on: ubuntu-x64
runs-on: ubuntu-latest
timeout-minutes: 10
env:
SLACK_CHANNEL_ID: plugins-notifications
Expand Down Expand Up @@ -226,7 +226,7 @@ jobs:
SLACK_BOT_TOKEN: ${{ secrets.TRUNKBOT_SLACK_BOT_TOKEN }}
generate_snapshots_pr:
name: Generate Snapshots PR
runs-on: ubuntu-x64
runs-on: ubuntu-latest
timeout-minutes: 30
needs: upload_test_results
if: needs.upload_test_results.outputs.reruns != ''
Expand Down
2 changes: 1 addition & 1 deletion .trunk/trunk.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ version: 0.1

# version used for local trunk runs and testing
cli:
version: 1.21.1-beta.16
version: 1.21.1-beta.17
shell_hooks:
enforce: true

Expand Down
2 changes: 1 addition & 1 deletion linters/brakeman/brakeman.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { osTimeoutMultiplier, skipOS, TEST_DATA } from "tests/utils";

// Note that the ruby setup can sometimes take a while.
// Ruby build is quite slow on Mac, so only run tests on linux for now
jest.setTimeout(600000 * osTimeoutMultiplier); // 300s or 900s
jest.setTimeout(600000 * osTimeoutMultiplier);

customLinterCheckTest({
linterName: "brakeman",
Expand Down
2 changes: 1 addition & 1 deletion linters/haml-lint/haml_lint.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { osTimeoutMultiplier, skipOS, TEST_DATA } from "tests/utils";

// Note that the first-time ruby/rufo download can sometimes take a while.
// Ruby build is quite slow on Mac, so only run tests on linux for now
jest.setTimeout(600000 * osTimeoutMultiplier); // 300s or 900s
jest.setTimeout(600000 * osTimeoutMultiplier);

// Ruby build is quite slow on Mac, so only run tests on linux for now
customLinterCheckTest({
Expand Down
2 changes: 1 addition & 1 deletion linters/kube-linter/kube_linter.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { linterCheckTest } from "tests";
import { osTimeoutMultiplier } from "tests/utils";

jest.setTimeout(900000 * osTimeoutMultiplier); // 300s or 900s
jest.setTimeout(900000 * osTimeoutMultiplier);

// TODO(Tyler): We will eventually need to add a couple more test cases involving failure modes.
linterCheckTest({ linterName: "kube-linter" });
2 changes: 1 addition & 1 deletion linters/nixpkgs-fmt/nixpkgs_fmt.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { linterFmtTest, TestCallback } from "tests";
import { osTimeoutMultiplier, skipOS } from "tests/utils";

jest.setTimeout(600000 * osTimeoutMultiplier); // 300s or 900s
jest.setTimeout(600000 * osTimeoutMultiplier);

// Earlier nixpkgs-fmt transitive dependencies are no longer
// supported through older rust runtime installs.
Expand Down
15 changes: 13 additions & 2 deletions linters/remark-lint/test_data/remark_lint_v11.0.0_basic.check.shot
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,24 @@ exports[`Testing linter remark-lint test basic 1`] = `
"issues": [
{
"code": "list-item-indent",
"column": "4",
"column": "1",
"file": "test_data/basic.in.md",
"issueClass": "ISSUE_CLASS_EXISTING",
"level": "LEVEL_HIGH",
"line": "1",
"linter": "remark-lint",
"message": "Unexpected \`2\` spaces between list item marker and content, expected \`1\` space, remove \`1\` space",
"targetType": "markdown",
},
{
"code": "list-item-indent",
"column": "1",
"file": "test_data/basic.in.md",
"issueClass": "ISSUE_CLASS_EXISTING",
"level": "LEVEL_HIGH",
"line": "1",
"linter": "remark-lint",
"message": "Incorrect list-item indent: add 1 space",
"message": "Unexpected \`3\` spaces between list item marker and content, expected \`1\` space, remove \`2\` spaces",
"targetType": "markdown",
},
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,18 @@ exports[`Testing linter remark-lint test basic 1`] = `
"level": "LEVEL_HIGH",
"line": "1",
"linter": "remark-lint",
"message": "Incorrect list-item indent: add 1 space",
"message": "Unexpected \`2\` spaces between list item marker and content, expected \`1\` space, remove \`1\` space",
"targetType": "markdown",
},
{
"code": "list-item-indent",
"column": "5",
"file": "test_data/basic.in.md",
"issueClass": "ISSUE_CLASS_EXISTING",
"level": "LEVEL_HIGH",
"line": "4",
"linter": "remark-lint",
"message": "Unexpected \`3\` spaces between list item marker and content, expected \`1\` space, remove \`2\` spaces",
"targetType": "markdown",
},
],
Expand Down
2 changes: 1 addition & 1 deletion linters/renovate/renovate.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { customLinterCheckTest } from "tests";
import { osTimeoutMultiplier, TEST_DATA } from "tests/utils";

jest.setTimeout(600000 * osTimeoutMultiplier); // 300s or 900s
jest.setTimeout(600000 * osTimeoutMultiplier);

customLinterCheckTest({
linterName: "renovate",
Expand Down
2 changes: 1 addition & 1 deletion linters/rubocop/rubocop.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { osTimeoutMultiplier, skipOS, TEST_DATA } from "tests/utils";

// Note that the first-time ruby/rufo download can sometimes take a while.
// Ruby build is quite slow on Mac, so only run tests on linux for now
jest.setTimeout(600000 * osTimeoutMultiplier); // 300s or 900s
jest.setTimeout(600000 * osTimeoutMultiplier);

const preCheck = (driver: TrunkLintDriver) => {
if (process.platform == "win32") {
Expand Down
2 changes: 1 addition & 1 deletion linters/rufo/rufo.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { osTimeoutMultiplier, skipOS, TEST_DATA } from "tests/utils";

// Note that the first-time ruby/rufo download can sometimes take a while.
// Ruby build is quite slow on Mac, so only run tests on linux for now
jest.setTimeout(600000 * osTimeoutMultiplier); // 300s or 900s
jest.setTimeout(600000 * osTimeoutMultiplier);

// Rufo succeeds on empty files
customLinterCheckTest({
Expand Down
2 changes: 1 addition & 1 deletion linters/standardrb/standardrb.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { osTimeoutMultiplier, skipOS } from "tests/utils";

// Note that the first-time ruby/rufo download can sometimes take a while.
// Ruby build is quite slow on Mac, so only run tests on linux for now
jest.setTimeout(600000 * osTimeoutMultiplier); // 300s or 900s
jest.setTimeout(600000 * osTimeoutMultiplier);

const preCheck = (driver: TrunkLintDriver) => {
if (process.platform == "win32") {
Expand Down
2 changes: 1 addition & 1 deletion tests/jest_setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { FileIssue } from "tests/types";
import { osTimeoutMultiplier } from "tests/utils";
import { isDeepStrictEqual } from "util";

jest.setTimeout(300000 * osTimeoutMultiplier); // 300s or 900s
jest.setTimeout(300000 * osTimeoutMultiplier);

/**
* Compares 2 file issues to determine exact equality.
Expand Down
2 changes: 1 addition & 1 deletion tests/repo_tests/config_check.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { osTimeoutMultiplier, REPO_ROOT } from "tests/utils";
// trunk-ignore-all(eslint/@typescript-eslint/no-unsafe-argument)
// trunk-ignore-all(eslint/@typescript-eslint/no-unsafe-return)

jest.setTimeout(300000 * osTimeoutMultiplier); // 300s or 900s
jest.setTimeout(300000 * osTimeoutMultiplier);

/**
* This test runs 'trunk config print' from the root of the repository to verify a healthy config.
Expand Down
1 change: 1 addition & 0 deletions tests/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ export interface LintAction {
parser: string;
report: string;
cacheHit?: boolean;
cacheExpiration?: string;
upstream: boolean;
fileGroupName: string;
command: string;
Expand Down
1 change: 1 addition & 0 deletions tests/utils/landing_state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const normalizePlatformPath = (originalPath: string | undefined) => {
const extractLintActionFields = ({
actionDurationMs: _actionDurationMs,
cacheHit: _cacheHit,
cacheExpiration: _cacheExpiration,
paths: _paths,
...rest
}: LintAction): LintAction => ({
Expand Down
4 changes: 4 additions & 0 deletions tools/goreleaser/goreleaser.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { toolInstallTest } from "tests";
import { osTimeoutMultiplier } from "tests/utils";

// This install is quite slow on some Linux machines.
jest.setTimeout(600000 * osTimeoutMultiplier);

toolInstallTest({
toolName: "goreleaser",
Expand Down
5 changes: 3 additions & 2 deletions tools/kubectl/kubectl.test.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
import { makeToolTestConfig, toolTest } from "tests";
import { skipOS } from "tests/utils";

toolTest({
toolName: "kubectl",
toolVersion: "1.25.16",
testConfigs: [
makeToolTestConfig({
// --short flag will be removed in future versions
command: ["kubectl", "version", "--short"],
expectedOut: "Client Version: v1.25.16",
// This fails on no kubectl credentials. Why do you need that for a version check? Who knows...
expectedExitCode: 1,
}),
],
skipTestIf: skipOS(["win32"]),
// kubectl does not run on Windows. But kubectl is frequently flaky in CI because it spins up a kubectl daemon.
skipTestIf: () => true,
});
2 changes: 1 addition & 1 deletion tools/renovate/renovate.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { makeToolTestConfig, toolTest } from "tests";
import { osTimeoutMultiplier, skipOS } from "tests/utils";

jest.setTimeout(600000 * osTimeoutMultiplier); // 300s or 900s
jest.setTimeout(600000 * osTimeoutMultiplier);

toolTest({
toolName: "renovate",
Expand Down

0 comments on commit 5611fee

Please sign in to comment.