From 0569164996f26fa850eebebbdea0fb897b73f1f8 Mon Sep 17 00:00:00 2001 From: Alexandr Yepishev Date: Tue, 26 Nov 2024 10:26:15 +0000 Subject: [PATCH] Update golangci-lint action to go through all dirs if no working directory set --- .github/actions/golangci-lint/action.yml | 18 ++++----- .github/workflows/ci-core.yml | 48 +++++++++++++++++------- .golangci.yml | 3 +- ccip/fail_test.go | 44 ++++++++++++++++++++++ core/fail_test.go | 47 +++++++++++++++++++++++ dashboard-lib/fail_test.go | 42 +++++++++++++++++++++ deployment/fail_test.go | 5 +++ 7 files changed, 182 insertions(+), 25 deletions(-) create mode 100644 ccip/fail_test.go create mode 100644 core/fail_test.go create mode 100644 dashboard-lib/fail_test.go diff --git a/.github/actions/golangci-lint/action.yml b/.github/actions/golangci-lint/action.yml index 20ad2689deb..73ae4add53b 100644 --- a/.github/actions/golangci-lint/action.yml +++ b/.github/actions/golangci-lint/action.yml @@ -42,10 +42,6 @@ runs: - name: Touching core/web/assets/index.html shell: bash run: mkdir -p core/web/assets && touch core/web/assets/index.html - - name: Build binary - working-directory: ${{ inputs.go-directory }} - shell: bash - run: go build ./... - name: Set golangci-lint working directory shell: bash id: set-working-directory @@ -59,18 +55,18 @@ runs: - name: golangci-lint uses: golangci/golangci-lint-action@38e1018663fa5173f3968ea0777460d3de38f256 # v5.3.0 with: - version: v1.61.0 + version: v1.62.2 only-new-issues: true - args: --out-format colored-line-number,checkstyle:golangci-lint-report.xml + args: --out-format colored-line-number,checkstyle:${{ steps.set-working-directory.outputs.golangci-lint-working-directory }}_golangci-lint-report.xml -v working-directory: ${{ steps.set-working-directory.outputs.golangci-lint-working-directory }} - name: Print lint report artifact if: failure() - shell: bash - run: cat ${{ inputs.go-directory }}/golangci-lint-report.xml + shell: bashc + run: cat ./${{ inputs.go-directory }}/${{ steps.set-working-directory.outputs.golangci-lint-working-directory }}_golangci-lint-report.xml - name: Store lint report artifact if: always() uses: actions/upload-artifact@v4.4.3 with: - name: golangci-lint-report - path: ${{ inputs.go-directory }}/golangci-lint-report.xml - retention-days: 7 + name: golangci-lint-report-${{ inputs.go-directory }} + # The ./ is needed to preserve the valid pattern for the artifact path + path: ./${{ inputs.go-directory }}/${{ inputs.go-directory }}_golangci-lint-report.xml diff --git a/.github/workflows/ci-core.yml b/.github/workflows/ci-core.yml index c38ecd918ae..9859c1ad895 100644 --- a/.github/workflows/ci-core.yml +++ b/.github/workflows/ci-core.yml @@ -33,6 +33,7 @@ jobs: permissions: pull-requests: read outputs: + affected-packages: ${{ steps.affected-modules.outputs.changes }} deployment-changes: ${{ steps.match-some.outputs.deployment == 'true' }} should-run-ci-core: ${{ steps.match-some.outputs.core-ci == 'true' || steps.match-every.outputs.non-ignored == 'true' || github.event_name == 'workflow_dispatch' || github.event_name == 'schedule' }} should-run-golangci: ${{ steps.match-some.outputs.golang-ci == 'true' || steps.match-every.outputs.non-ignored == 'true' || github.event_name == 'workflow_dispatch' }} @@ -47,7 +48,7 @@ jobs: with: # "if any changed file matches one or more of the conditions" (https://github.com/dorny/paths-filter/issues/225) predicate-quantifier: some - # deployment - any changes to files in `deployments/` + # deployment - any changes to files in the `deployments/` # core-ci - any changes that could affect this workflow definition # golang-ci - any changes that could affect the linting result filters: | @@ -75,6 +76,7 @@ jobs: non-ignored: - '**' - '!docs/**' + - '!fuzz/**' - '!integration-tests/**' - '!tools/secrets/**' - '!tools/goreleaser-config/**' @@ -91,24 +93,44 @@ jobs: - '!nix-darwin-shell-hook.sh' - '!LICENSE' - '!.github/**' - + - uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2 + id: affected-modules + with: + # to get affected packages as output (not simply `true` or `false`) + # use the following syntax `package_name: 'path/to/package'` + filters: | + ccip: 'ccip/**' + common: 'common/**' + core: 'core/**' + dashboard-lib: 'dashboard-lib/**' + deployment: 'deployment/**' + plugins: 'plugins/**' + golangci: - # We don't directly merge dependabot PRs, so let's not waste the resources - if: ${{ (github.event_name == 'pull_request' || github.event_name == 'schedule') && github.actor != 'dependabot[bot]' }} name: lint + # We don't directly merge dependabot PRs, so let's not waste the resources. + # toJson/fromJson is used to account for empty array formatting differences ([], [ ], etc.) + # if: ${{ (github.event_name == 'pull_request' || github.event_name == 'schedule') && github.actor != 'dependabot[bot]' && needs.filter.outputs.should-run-golangci == 'true' && (toJson(fromJson(needs.filter.outputs.affected-packages)) != '[]' && needs.filter.outputs.affected-packages != '')}} + if: always() + needs: [filter, run-frequency] permissions: - # For golangci-lint-actions to annotate code in the PR. + # To annotate code in the PR. checks: write contents: read # For golangci-lint-action's `only-new-issues` option. pull-requests: read runs-on: ubuntu-24.04-8cores-32GB-ARM - needs: [filter, run-frequency] + strategy: + fail-fast: false + matrix: + modules: ${{ fromJson(needs.filter.outputs.affected-packages) }} steps: - - uses: actions/checkout@v4.2.1 - - name: Golang Lint + - name: Checkout + uses: actions/checkout@v4.2.1 + - name: Golang Lint (${{ matrix.modules }}) uses: ./.github/actions/golangci-lint - if: ${{ needs.filter.outputs.should-run-golangci == 'true' }} + with: + go-directory: ${{ matrix.modules }} - name: Notify Slack if: ${{ failure() && needs.run-frequency.outputs.one-per-day-frequency == 'true' }} uses: slackapi/slack-github-action@6c661ce58804a1a20f6dc5fbee7f0381b469e001 # v1.25.0 @@ -375,8 +397,8 @@ jobs: scan: name: SonarQube Scan - needs: [core, run-frequency] - if: ${{ always() && needs.run-frequency.outputs.four-per-day-frequency == 'true' && github.actor != 'dependabot[bot]' }} + needs: [core] + if: ${{ always() && github.actor != 'dependabot[bot]' }} runs-on: ubuntu-latest steps: - name: Checkout the repo @@ -384,7 +406,7 @@ jobs: with: fetch-depth: 0 # fetches all history for all tags and branches to provide more metadata for sonar reports - - name: Download all workflow run artifacts + - name: Download all workflow artifacts uses: actions/download-artifact@v4.1.8 - name: Check and Set SonarQube Report Paths @@ -410,7 +432,7 @@ jobs: # Check and assign paths for lint reports if [ -d "golangci-lint-report" ]; then - sonarqube_lint_report_paths=$(find golangci-lint-report -name golangci-lint-report.xml | paste -sd "," -) + sonarqube_lint_report_paths=$(find -type f -name 'golangci-lint-report.xml' -printf "%p,") else sonarqube_lint_report_paths="" fi diff --git a/.golangci.yml b/.golangci.yml index ca8cf4dade5..2937bae43bb 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,8 +1,9 @@ run: timeout: 15m0s + allow-parallel-runners: true + allow-serial-runners: true linters: enable: - - containedctx - depguard - errname - errorlint diff --git a/ccip/fail_test.go b/ccip/fail_test.go new file mode 100644 index 00000000000..498ad4ad5e5 --- /dev/null +++ b/ccip/fail_test.go @@ -0,0 +1,44 @@ +package ccip + +import ( + "errors" + "os" + "sync" + "testing" +) + +func TestFail(t *testing.T) { + if testing.Short() { + t.Skip() + } + t.Fatal("fake failure") +} + +func TestRace(t *testing.T) { + var v int + var wg sync.WaitGroup + wg.Add(100) + for i := 0; i < 100; i++ { + go func() { + defer wg.Done() + v++ + v-- + }() + } + wg.Wait() + t.Log(v) +} + +func TestLint(t *testing.T) { + const v1 = (true && false) && (true && false) // SQ Identical expressions should not be used on both sides of a binary operator + a := 1 + if !(a == 2) { // SQ boolean check should not be inverted + } + const UnusedVar = 1 // lint should complain for unused variable + const ALL_CAPS = 10 // should be AllCaps + err := os.ErrNotExist + if err == os.ErrNotExist { // should use errors.Is + err := errors.New("fake error") // shadowed variable + t.Log(err) + } +} diff --git a/core/fail_test.go b/core/fail_test.go new file mode 100644 index 00000000000..87f1d97daa2 --- /dev/null +++ b/core/fail_test.go @@ -0,0 +1,47 @@ +package core + +import ( + "context" + "sync" + "testing" +) + +type MyStruct struct { + Ctx context.Context // triggers `containedctx`` rule +} + +func TestFail(t *testing.T) { + if testing.Short() { + t.Skip() + } + t.Fatal("fake failure") +} + +func TestRace(t *testing.T) { + var v int + var wg sync.WaitGroup + wg.Add(100) + for i := 0; i < 100; i++ { + go func() { + defer wg.Done() + v++ + v-- + }() + } + wg.Wait() + t.Log(v) +} + +// func TestLint(t *testing.T) { +// const v1 = (true && false) && (true && false) // SQ Identical expressions should not be used on both sides of a binary operator +// a := 1 +// if !(a == 2) { // SQ boolean check should not be inverted +// } +// const UnusedVar = 1 // lint should complain for unused variable +// const ALL_CAPS = 10 // should be AllCaps +// err := os.ErrNotExist +// if err == os.ErrNotExist { // should use errors.Is +// err := errors.New("fake error") // shadowed variable +// t.Log(err) +// } +// } diff --git a/dashboard-lib/fail_test.go b/dashboard-lib/fail_test.go new file mode 100644 index 00000000000..5944f0992e5 --- /dev/null +++ b/dashboard-lib/fail_test.go @@ -0,0 +1,42 @@ +package dashboard_lib + +import ( + "sync" + "testing" +) + +func TestFail(t *testing.T) { + if testing.Short() { + t.Skip() + } + t.Fatal("fake failure") +} + +func TestRace(t *testing.T) { + var v int + var wg sync.WaitGroup + wg.Add(100) + for i := 0; i < 100; i++ { + go func() { + defer wg.Done() + v++ + v-- + }() + } + wg.Wait() + t.Log(v) +} + +// func TestLint(t *testing.T) { +// const v1 = (true && false) && (true && false) // SQ Identical expressions should not be used on both sides of a binary operator +// a := 1 +// if !(a == 2) { // SQ boolean check should not be inverted +// } +// const UnusedVar = 1 // lint should complain for unused variable +// const ALL_CAPS = 10 // should be AllCaps +// err := os.ErrNotExist +// if err == os.ErrNotExist { // should use errors.Is +// err := errors.New("fake error") // shadowed variable +// t.Log(err) +// } +// } diff --git a/deployment/fail_test.go b/deployment/fail_test.go index c9c78e5091c..1018b4be738 100644 --- a/deployment/fail_test.go +++ b/deployment/fail_test.go @@ -1,12 +1,17 @@ package deployment import ( + "context" "errors" "os" "sync" "testing" ) +type MyStruct struct { + Ctx context.Context // triggers context rules +} + func TestFail(t *testing.T) { if testing.Short() { t.Skip()