From 60da77f8fb529674985c94ee56b8bcb3d5d52e27 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Tue, 12 Aug 2025 10:34:12 +0200 Subject: [PATCH 1/4] ci: Add unit tests required check Use dorny path filters to have one single job failing or succeeding when the unit tests passed or were skipped so we can enable unit tests as required checks for PRs. Fixes GH-5868 --- .github/file-filters.yml | 17 +++++++++++ .github/workflows/test.yml | 59 +++++++++++++++++++++++++++----------- 2 files changed, 60 insertions(+), 16 deletions(-) diff --git a/.github/file-filters.yml b/.github/file-filters.yml index d08ff897b71..ed27a27dcc4 100644 --- a/.github/file-filters.yml +++ b/.github/file-filters.yml @@ -15,3 +15,20 @@ high_risk_code: &high_risk_code - "Sources/Sentry/SentryFileManager.m" - "Sources/Sentry/SentrySerialization.m" - "Sources/Sentry/SentrySerialization.h" + +run_unit_tests_for_prs: &run_unit_tests_for_prs + - "Sources/**" + - "Tests/**" + - "SentryTestUtils/**" + - "SentryTestUtilsDynamic/**" + - "test-server/**" + - ".github/workflows/test.yml" + - "fastlane/**" + - "scripts/tests-with-thread-sanitizer.sh" + - "scripts/ci-select-xcode.sh" + - "scripts/sentry-xcodebuild.sh" + - ".codecov.yml" + - "Sentry.xcodeproj" + - "**/*.xctestplan" + - "Makefile" # Make commands used for CI build setup + - "Brewfile*" # Dependency installation affects test environment diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 177e5ac73df..cfd00ccf549 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -6,22 +6,6 @@ on: - release/** pull_request: - paths: - - "Sources/**" - - "Tests/**" - - "SentryTestUtils/**" - - "SentryTestUtilsDynamic/**" # Dynamic test utilities used by tests - - "test-server/**" - - ".github/workflows/test.yml" - - "fastlane/**" - - "scripts/tests-with-thread-sanitizer.sh" - - "scripts/ci-select-xcode.sh" - - "scripts/sentry-xcodebuild.sh" - - ".codecov.yml" - - "Sentry.xcodeproj" - - "**/*.xctestplan" - - "Makefile" # Make commands used for CI build setup - - "Brewfile*" # Dependency installation affects test environment # https://docs.github.com/en/actions/using-jobs/using-concurrency#example-using-a-fallback-value concurrency: @@ -29,8 +13,29 @@ concurrency: cancel-in-progress: true jobs: + # This job detects if the PR contains changes that require running unit tests. + # If yes, the job will output a flag that will be used by the next job to run the unit tests. + # If no, the job will output a flag that will be used by the next job to skip running the unit tests. + # At the end of this workflow, we run a check that validates that either all unit tests passed or were + # called unit-tests-required-check. + files-changed: + name: Detect File Changes + runs-on: ubuntu-latest + # Map a step output to a job output + outputs: + run_unit_tests_for_prs: ${{ steps.changes.outputs.run_unit_tests_for_prs }} + steps: + - uses: actions/checkout@v4 + - name: Get changed files + id: changes + uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2 + with: + token: ${{ github.token }} + filters: .github/file-filters.yml build-test-server: name: Build test server + if: needs.files-changed.outputs.run_unit_tests_for_prs == 'true' + needs: files-changed runs-on: macos-15 steps: - uses: actions/checkout@v4 @@ -300,3 +305,25 @@ jobs: verbose: true name: sentry-cocoa-unit-tests flags: unittests-${{ matrix.platform }}-${{ matrix.xcode }}-${{ matrix.test-destination-os }}, unittests + + # This check validates that either all unit tests passed or were skipped, which allows us + # to make unit tests a required check with only running the unit tests when required. + # So, we don't have to run unit tests, for example, for Changelog or ReadMe changes. + + unit-tests-required-check: + needs: + [ + build-test-server, + unit-tests, + ] + name: Unit Tests + # This is necessary since a failed/skipped dependent job would cause this job to be skipped + if: always() + runs-on: ubuntu-latest + steps: + # If any jobs we depend on fails gets cancelled or times out, this job will fail. + # Skipped jobs are not considered failures. + - name: Check for failures + if: contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') + run: | + echo "One of the unit test jobs has failed." && exit 1 From 0ea6f56f3bf875ef4d2d7815d3d1624e9447daa4 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Tue, 12 Aug 2025 10:38:00 +0200 Subject: [PATCH 2/4] sort and group file filters --- .github/file-filters.yml | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/.github/file-filters.yml b/.github/file-filters.yml index ed27a27dcc4..3454294ef71 100644 --- a/.github/file-filters.yml +++ b/.github/file-filters.yml @@ -16,19 +16,26 @@ high_risk_code: &high_risk_code - "Sources/Sentry/SentrySerialization.m" - "Sources/Sentry/SentrySerialization.h" -run_unit_tests_for_prs: &run_unit_tests_for_prs +run_unit_tests_for_prs: &run_unit_tests_for_prs # Code - "Sources/**" - "Tests/**" - "SentryTestUtils/**" - "SentryTestUtilsDynamic/**" - - "test-server/**" + + # GH Actions - ".github/workflows/test.yml" - - "fastlane/**" - - "scripts/tests-with-thread-sanitizer.sh" + - ".github/file-filters.yml" + + # Scripts - "scripts/ci-select-xcode.sh" - "scripts/sentry-xcodebuild.sh" - - ".codecov.yml" + - "scripts/tests-with-thread-sanitizer.sh" + + # Other + - "test-server/**" - "Sentry.xcodeproj" - "**/*.xctestplan" - - "Makefile" # Make commands used for CI build setup + - "fastlane/**" + - ".codecov.yml" - "Brewfile*" # Dependency installation affects test environment + - "Makefile" # Make commands used for CI build setup From f54fdca49870c5531263e45c1c7b80ea0f6c93ac Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Tue, 12 Aug 2025 13:16:52 +0200 Subject: [PATCH 3/4] let some tests flake --- Tests/SentryTests/SentrySDKTests.swift | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Tests/SentryTests/SentrySDKTests.swift b/Tests/SentryTests/SentrySDKTests.swift index ac42cc755fa..839fa840fdc 100644 --- a/Tests/SentryTests/SentrySDKTests.swift +++ b/Tests/SentryTests/SentrySDKTests.swift @@ -396,6 +396,12 @@ class SentrySDKTests: XCTestCase { SentrySDK.capture(message: fixture.message, block: fixture.scopeBlock) assertHubScopeNotChanged() + + // Intentionally fail about 30% of the time to simulate flakiness for CI experiments + if Int.random(in: 0..<100) < 30 { + XCTFail("Injected random failure (30%) for flakiness testing") + } + } } From e0e10cc61e4f6f27d2754452e87c423530b59ca6 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Tue, 12 Aug 2025 14:48:18 +0200 Subject: [PATCH 4/4] remove failing unit test --- Tests/SentryTests/SentrySDKTests.swift | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Tests/SentryTests/SentrySDKTests.swift b/Tests/SentryTests/SentrySDKTests.swift index 839fa840fdc..ac42cc755fa 100644 --- a/Tests/SentryTests/SentrySDKTests.swift +++ b/Tests/SentryTests/SentrySDKTests.swift @@ -396,12 +396,6 @@ class SentrySDKTests: XCTestCase { SentrySDK.capture(message: fixture.message, block: fixture.scopeBlock) assertHubScopeNotChanged() - - // Intentionally fail about 30% of the time to simulate flakiness for CI experiments - if Int.random(in: 0..<100) < 30 { - XCTFail("Injected random failure (30%) for flakiness testing") - } - } }