From 6976208e21436e88a4a5a094b440d41c482c5c84 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Thu, 18 Dec 2025 15:23:09 -0700 Subject: [PATCH] Move autofix stuff to zippy (#45304) Although I wanted to avoid the dependency, it's hard to get github to do what we want. Release Notes: - N/A --- .github/workflows/extension_tests.yml | 3 +- .github/workflows/release.yml | 16 +--- .github/workflows/release_nightly.yml | 6 +- .github/workflows/run_tests.yml | 47 ++---------- .../xtask/src/tasks/workflows/run_tests.rs | 73 +------------------ tooling/xtask/src/tasks/workflows/steps.rs | 33 +-------- 6 files changed, 19 insertions(+), 159 deletions(-) diff --git a/.github/workflows/extension_tests.yml b/.github/workflows/extension_tests.yml index 7a7fff9b97d694c1b02dd426f5d59301fe2be81e..9f0917e388c74cffed8f342f7504bc111e6f5147 100644 --- a/.github/workflows/extension_tests.yml +++ b/.github/workflows/extension_tests.yml @@ -61,8 +61,7 @@ jobs: uses: namespacelabs/nscloud-cache-action@v1 with: cache: rust - - id: cargo_fmt - name: steps::cargo_fmt + - name: steps::cargo_fmt run: cargo fmt --all -- --check shell: bash -euxo pipefail {0} - name: extension_tests::run_clippy diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 317d5a8df37a62887ce4ddcdd67c8d77b48d56d6..ffc2554a55e00a5bdb7bd1ee0bfeebd5667755d5 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -26,8 +26,7 @@ jobs: uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 with: node-version: '20' - - id: clippy - name: steps::clippy + - name: steps::clippy run: ./script/clippy shell: bash -euxo pipefail {0} - name: steps::clear_target_dir_if_large @@ -72,15 +71,9 @@ jobs: uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 with: node-version: '20' - - id: clippy - name: steps::clippy + - name: steps::clippy run: ./script/clippy shell: bash -euxo pipefail {0} - - id: record_clippy_failure - name: steps::record_clippy_failure - if: always() - run: echo "failed=${{ steps.clippy.outcome == 'failure' }}" >> "$GITHUB_OUTPUT" - shell: bash -euxo pipefail {0} - name: steps::cargo_install_nextest uses: taiki-e/install-action@nextest - name: steps::clear_target_dir_if_large @@ -94,8 +87,6 @@ jobs: run: | rm -rf ./../.cargo shell: bash -euxo pipefail {0} - outputs: - clippy_failed: ${{ steps.record_clippy_failure.outputs.failed == 'true' }} timeout-minutes: 60 run_tests_windows: if: (github.repository_owner == 'zed-industries' || github.repository_owner == 'zed-extensions') @@ -114,8 +105,7 @@ jobs: uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 with: node-version: '20' - - id: clippy - name: steps::clippy + - name: steps::clippy run: ./script/clippy.ps1 shell: pwsh - name: steps::clear_target_dir_if_large diff --git a/.github/workflows/release_nightly.yml b/.github/workflows/release_nightly.yml index b23e4b7518a672c0d586ea5ba437db5cf8f94bb6..d76244175accc3e816cbd7d5dc322d2529a0a236 100644 --- a/.github/workflows/release_nightly.yml +++ b/.github/workflows/release_nightly.yml @@ -20,8 +20,7 @@ jobs: with: clean: false fetch-depth: 0 - - id: cargo_fmt - name: steps::cargo_fmt + - name: steps::cargo_fmt run: cargo fmt --all -- --check shell: bash -euxo pipefail {0} - name: ./script/clippy @@ -45,8 +44,7 @@ jobs: uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 with: node-version: '20' - - id: clippy - name: steps::clippy + - name: steps::clippy run: ./script/clippy.ps1 shell: pwsh - name: steps::clear_target_dir_if_large diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index fac3221d63a080fa53b7ba1c5b7249e6a405c73c..256bb2916a56485c06c2ebc4de8724151d622c4f 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -74,19 +74,12 @@ jobs: uses: pnpm/action-setup@fe02b34f77f8bc703788d5817da081398fad5dd2 with: version: '9' - - id: prettier - name: steps::prettier + - name: steps::prettier run: ./script/prettier shell: bash -euxo pipefail {0} - - id: cargo_fmt - name: steps::cargo_fmt + - name: steps::cargo_fmt run: cargo fmt --all -- --check shell: bash -euxo pipefail {0} - - id: record_style_failure - name: steps::record_style_failure - if: always() - run: echo "failed=${{ steps.prettier.outcome == 'failure' || steps.cargo_fmt.outcome == 'failure' }}" >> "$GITHUB_OUTPUT" - shell: bash -euxo pipefail {0} - name: ./script/check-todos run: ./script/check-todos shell: bash -euxo pipefail {0} @@ -97,8 +90,6 @@ jobs: uses: crate-ci/typos@2d0ce569feab1f8752f1dde43cc2f2aa53236e06 with: config: ./typos.toml - outputs: - style_failed: ${{ steps.record_style_failure.outputs.failed == 'true' }} timeout-minutes: 60 run_tests_windows: needs: @@ -119,8 +110,7 @@ jobs: uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 with: node-version: '20' - - id: clippy - name: steps::clippy + - name: steps::clippy run: ./script/clippy.ps1 shell: pwsh - name: steps::clear_target_dir_if_large @@ -167,15 +157,9 @@ jobs: uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 with: node-version: '20' - - id: clippy - name: steps::clippy + - name: steps::clippy run: ./script/clippy shell: bash -euxo pipefail {0} - - id: record_clippy_failure - name: steps::record_clippy_failure - if: always() - run: echo "failed=${{ steps.clippy.outcome == 'failure' }}" >> "$GITHUB_OUTPUT" - shell: bash -euxo pipefail {0} - name: steps::cargo_install_nextest uses: taiki-e/install-action@nextest - name: steps::clear_target_dir_if_large @@ -189,8 +173,6 @@ jobs: run: | rm -rf ./../.cargo shell: bash -euxo pipefail {0} - outputs: - clippy_failed: ${{ steps.record_clippy_failure.outputs.failed == 'true' }} timeout-minutes: 60 run_tests_mac: needs: @@ -211,8 +193,7 @@ jobs: uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 with: node-version: '20' - - id: clippy - name: steps::clippy + - name: steps::clippy run: ./script/clippy shell: bash -euxo pipefail {0} - name: steps::clear_target_dir_if_large @@ -592,24 +573,6 @@ jobs: exit $EXIT_CODE shell: bash -euxo pipefail {0} - call_autofix: - needs: - - check_style - - run_tests_linux - if: always() && (needs.check_style.outputs.style_failed == 'true' || needs.run_tests_linux.outputs.clippy_failed == 'true') && github.event_name == 'pull_request' && github.actor != 'zed-zippy[bot]' - runs-on: namespace-profile-2x4-ubuntu-2404 - steps: - - id: get-app-token - name: steps::authenticate_as_zippy - uses: actions/create-github-app-token@bef1eaf1c0ac2b148ee2a0a74c65fbe6db0631f1 - with: - app-id: ${{ secrets.ZED_ZIPPY_APP_ID }} - private-key: ${{ secrets.ZED_ZIPPY_APP_PRIVATE_KEY }} - - name: run_tests::call_autofix::dispatch_autofix - run: gh workflow run autofix_pr.yml -f pr_number=${{ github.event.pull_request.number }} -f run_clippy=${{ needs.run_tests_linux.outputs.clippy_failed == 'true' }} - shell: bash -euxo pipefail {0} - env: - GITHUB_TOKEN: ${{ steps.get-app-token.outputs.token }} concurrency: group: ${{ github.workflow }}-${{ github.ref_name }}-${{ github.ref_name == 'main' && github.sha || 'anysha' }} cancel-in-progress: true diff --git a/tooling/xtask/src/tasks/workflows/run_tests.rs b/tooling/xtask/src/tasks/workflows/run_tests.rs index d0caab82b057f21735b7f828c8917a358dd548b2..f726f48740eb7819fbbd3fed369e5e4e89c526c9 100644 --- a/tooling/xtask/src/tasks/workflows/run_tests.rs +++ b/tooling/xtask/src/tasks/workflows/run_tests.rs @@ -45,15 +45,11 @@ pub(crate) fn run_tests() -> Workflow { &should_run_tests, ]); - let check_style = check_style(); - let run_tests_linux = run_platform_tests(Platform::Linux); - let call_autofix = call_autofix(&check_style, &run_tests_linux); - let mut jobs = vec![ orchestrate, - check_style, + check_style(), should_run_tests.guard(run_platform_tests(Platform::Windows)), - should_run_tests.guard(run_tests_linux), + should_run_tests.guard(run_platform_tests(Platform::Linux)), should_run_tests.guard(run_platform_tests(Platform::Mac)), should_run_tests.guard(doctests()), should_run_tests.guard(check_workspace_binaries()), @@ -110,7 +106,6 @@ pub(crate) fn run_tests() -> Workflow { workflow }) .add_job(tests_pass.name, tests_pass.job) - .add_job(call_autofix.name, call_autofix.job) } // Generates a bash script that checks changed files against regex patterns @@ -226,8 +221,6 @@ pub fn tests_pass(jobs: &[NamedJob]) -> NamedJob { named::job(job) } -pub const STYLE_FAILED_OUTPUT: &str = "style_failed"; - fn check_style() -> NamedJob { fn check_for_typos() -> Step { named::uses( @@ -245,56 +238,12 @@ fn check_style() -> NamedJob { .add_step(steps::setup_pnpm()) .add_step(steps::prettier()) .add_step(steps::cargo_fmt()) - .add_step(steps::record_style_failure()) .add_step(steps::script("./script/check-todos")) .add_step(steps::script("./script/check-keymaps")) - .add_step(check_for_typos()) - .outputs([( - STYLE_FAILED_OUTPUT.to_owned(), - format!( - "${{{{ steps.{}.outputs.failed == 'true' }}}}", - steps::RECORD_STYLE_FAILURE_STEP_ID - ), - )]), + .add_step(check_for_typos()), ) } -fn call_autofix(check_style: &NamedJob, run_tests_linux: &NamedJob) -> NamedJob { - fn dispatch_autofix(run_tests_linux_name: &str) -> Step { - let clippy_failed_expr = format!( - "needs.{}.outputs.{} == 'true'", - run_tests_linux_name, CLIPPY_FAILED_OUTPUT - ); - named::bash(format!( - "gh workflow run autofix_pr.yml -f pr_number=${{{{ github.event.pull_request.number }}}} -f run_clippy=${{{{ {} }}}}", - clippy_failed_expr - )) - .add_env(("GITHUB_TOKEN", "${{ steps.get-app-token.outputs.token }}")) - } - - let style_failed_expr = format!( - "needs.{}.outputs.{} == 'true'", - check_style.name, STYLE_FAILED_OUTPUT - ); - let clippy_failed_expr = format!( - "needs.{}.outputs.{} == 'true'", - run_tests_linux.name, CLIPPY_FAILED_OUTPUT - ); - let (authenticate, _token) = steps::authenticate_as_zippy(); - - let job = Job::default() - .runs_on(runners::LINUX_SMALL) - .cond(Expression::new(format!( - "always() && ({} || {}) && github.event_name == 'pull_request' && github.actor != 'zed-zippy[bot]'", - style_failed_expr, clippy_failed_expr - ))) - .needs(vec![check_style.name.clone(), run_tests_linux.name.clone()]) - .add_step(authenticate) - .add_step(dispatch_autofix(&run_tests_linux.name)); - - named::job(job) -} - fn check_dependencies() -> NamedJob { fn install_cargo_machete() -> Step { named::uses( @@ -355,8 +304,6 @@ fn check_workspace_binaries() -> NamedJob { ) } -pub const CLIPPY_FAILED_OUTPUT: &str = "clippy_failed"; - pub(crate) fn run_platform_tests(platform: Platform) -> NamedJob { let runner = match platform { Platform::Windows => runners::WINDOWS_DEFAULT, @@ -378,24 +325,12 @@ pub(crate) fn run_platform_tests(platform: Platform) -> NamedJob { ) .add_step(steps::setup_node()) .add_step(steps::clippy(platform)) - .when(platform == Platform::Linux, |job| { - job.add_step(steps::record_clippy_failure()) - }) .when(platform == Platform::Linux, |job| { job.add_step(steps::cargo_install_nextest()) }) .add_step(steps::clear_target_dir_if_large(platform)) .add_step(steps::cargo_nextest(platform)) - .add_step(steps::cleanup_cargo_config(platform)) - .when(platform == Platform::Linux, |job| { - job.outputs([( - CLIPPY_FAILED_OUTPUT.to_owned(), - format!( - "${{{{ steps.{}.outputs.failed == 'true' }}}}", - steps::RECORD_CLIPPY_FAILURE_STEP_ID - ), - )]) - }), + .add_step(steps::cleanup_cargo_config(platform)), } } diff --git a/tooling/xtask/src/tasks/workflows/steps.rs b/tooling/xtask/src/tasks/workflows/steps.rs index eaa51dc35205f51e7fe3a56668ed0679e92999f0..a0b071cd6c31654b42adddbba47dd24c60da7df2 100644 --- a/tooling/xtask/src/tasks/workflows/steps.rs +++ b/tooling/xtask/src/tasks/workflows/steps.rs @@ -54,25 +54,12 @@ pub fn setup_sentry() -> Step { .add_with(("token", vars::SENTRY_AUTH_TOKEN)) } -pub const PRETTIER_STEP_ID: &str = "prettier"; -pub const CARGO_FMT_STEP_ID: &str = "cargo_fmt"; -pub const RECORD_STYLE_FAILURE_STEP_ID: &str = "record_style_failure"; - pub fn prettier() -> Step { - named::bash("./script/prettier").id(PRETTIER_STEP_ID) + named::bash("./script/prettier") } pub fn cargo_fmt() -> Step { - named::bash("cargo fmt --all -- --check").id(CARGO_FMT_STEP_ID) -} - -pub fn record_style_failure() -> Step { - named::bash(format!( - "echo \"failed=${{{{ steps.{}.outcome == 'failure' || steps.{}.outcome == 'failure' }}}}\" >> \"$GITHUB_OUTPUT\"", - PRETTIER_STEP_ID, CARGO_FMT_STEP_ID - )) - .id(RECORD_STYLE_FAILURE_STEP_ID) - .if_condition(Expression::new("always()")) + named::bash("cargo fmt --all -- --check") } pub fn cargo_install_nextest() -> Step { @@ -118,25 +105,13 @@ pub fn clear_target_dir_if_large(platform: Platform) -> Step { } } -pub const CLIPPY_STEP_ID: &str = "clippy"; -pub const RECORD_CLIPPY_FAILURE_STEP_ID: &str = "record_clippy_failure"; - pub fn clippy(platform: Platform) -> Step { match platform { - Platform::Windows => named::pwsh("./script/clippy.ps1").id(CLIPPY_STEP_ID), - _ => named::bash("./script/clippy").id(CLIPPY_STEP_ID), + Platform::Windows => named::pwsh("./script/clippy.ps1"), + _ => named::bash("./script/clippy"), } } -pub fn record_clippy_failure() -> Step { - named::bash(format!( - "echo \"failed=${{{{ steps.{}.outcome == 'failure' }}}}\" >> \"$GITHUB_OUTPUT\"", - CLIPPY_STEP_ID - )) - .id(RECORD_CLIPPY_FAILURE_STEP_ID) - .if_condition(Expression::new("always()")) -} - pub fn cache_rust_dependencies_namespace() -> Step { named::uses("namespacelabs", "nscloud-cache-action", "v1").add_with(("cache", "rust")) }