Skip to content
Draft
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .github/file-filters.yml
Original file line number Diff line number Diff line change
Expand Up @@ -541,3 +541,11 @@ run_size_analysis_for_prs: &run_size_analysis_for_prs
# Build configuration
- "Makefile"
- "Brewfile*"

run_swift_log_for_prs: &run_swift_log_for_prs
- "integrations/logs/sentry-cocoa-swiftlog/**"

# GH Actions
- ".github/workflows/integrations-common.yml"
- ".github/workflows/integrations-swift-log.yml"
- ".github/file-filters.yml"
134 changes: 134 additions & 0 deletions .github/workflows/integrations-swift-log.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
name: Test Integrations
on:
push:
branches:
- main
- release/**

pull_request:
types: [opened, synchronize, reopened, labeled]

# Concurrency configuration:
# - We use workflow-specific concurrency groups to allow independent test runs across different workflows
# while preventing multiple runs of the same test suite on the same branch/commit.
# - For pull requests, we cancel in-progress runs when new commits are pushed to save CI resources
# and provide faster feedback on the latest changes.
# - For main branch pushes and scheduled runs, we never cancel in-progress runs to ensure the complete
# test suite always finishes, maintaining the integrity of our main branch quality gates.
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: ${{ github.event_name == 'pull_request' }}

jobs:
ready-to-merge-gate:
name: Ready-to-merge gate
uses: ./.github/workflows/ready-to-merge-workflow.yml

# 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
needs: ready-to-merge-gate
# Map a step output to a job output
outputs:
run_swift_log_for_prs: ${{ steps.changes.outputs.run_swift_log_for_prs }}
steps:
- uses: actions/checkout@v6
- name: Get changed files
id: changes
uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2
with:
token: ${{ github.token }}
filters: .github/file-filters.yml

spm-integrations-tests:
name: "SPM Tests for Swift Log"
needs: files-changed
if: github.event_name != 'pull_request' || needs.files-changed.outputs.run_swift_log_for_prs == 'true'
runs-on: macos-15
timeout-minutes: 30
steps:
- uses: actions/checkout@v6
- name: Select Xcode version
env:
XCODE_VERSION: 16.4
run: ./scripts/ci-select-xcode.sh "$XCODE_VERSION"
- name: Prepare Package.swift
shell: bash
run: |
./scripts/prepare-package.sh \
--package-file integrations/logs/sentry-cocoa-swiftlog/Package.swift \
--update-path-to-sentry-cocoa true
- name: Run SPM Tests
working-directory: integrations/logs/sentry-cocoa-swiftlog
run: swift test

test-integration:
name: Unit Tests for Swift Log
if: github.event_name != 'pull_request' || needs.files-changed.outputs.run_swift_log_for_prs == 'true'
needs: files-changed
uses: ./.github/workflows/unit-test-common.yml
with:
name: SwiftLog
scheme: SentrySwiftLog
working_directory: integrations/logs/sentry-cocoa-swiftlog
path_to_scripts_folder: ../../../scripts
xcode: ${{ matrix.xcode }}
test-destination-os: ${{ matrix.test-destination-os }}
device: ${{ matrix.device }}
platform: ${{ matrix.platform }}
runs-on: ${{ matrix.macos_version }}
Comment on lines 76 to 86
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: sentry-xcodebuild.sh incorrectly adds -workspace Sentry.xcworkspace when testing SPM packages due to a missing --spm-project flag.
Severity: CRITICAL | Confidence: High

🔍 Detailed Analysis

The integrations-swift-log.yml workflow calls unit-test-common.yml to test an SPM package. However, unit-test-common.yml does not expose an input parameter to indicate an SPM project. Consequently, sentry-xcodebuild.sh is executed without the --spm-project true flag, causing it to incorrectly add -workspace Sentry.xcworkspace to xcodebuild arguments. This will fail because the SPM package directory (integrations/logs/sentry-cocoa-swiftlog/) does not contain a Sentry.xcworkspace file.

💡 Suggested Fix

Add an is_spm_project boolean input to unit-test-common.yml, pass it to sentry-xcodebuild.sh, and update integrations-swift-log.yml to pass is_spm_project: true.

🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: .github/workflows/integrations-swift-log.yml#L71-L85

Potential issue: The `integrations-swift-log.yml` workflow calls `unit-test-common.yml`
to test an SPM package. However, `unit-test-common.yml` does not expose an input
parameter to indicate an SPM project. Consequently, `sentry-xcodebuild.sh` is executed
without the `--spm-project true` flag, causing it to incorrectly add `-workspace
Sentry.xcworkspace` to `xcodebuild` arguments. This will fail because the SPM package
directory (`integrations/logs/sentry-cocoa-swiftlog/`) does not contain a
`Sentry.xcworkspace` file.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 5615901

timeout: 10
Comment on lines 77 to 87

This comment was marked as outdated.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Missing secrets inheritance for codecov token access

The test-integration job is missing secrets: inherit when calling the unit-test-common.yml reusable workflow. Other callers like test.yml and fast-pr-checks.yml include this directive. Without it, unit-test-common.yml cannot access secrets.CODECOV_TOKEN, causing the codecov upload steps to fail or behave unexpectedly.

Fix in Cursor Fix in Web

strategy:
fail-fast: false
matrix:
include:
- name: iOS 18
xcode: "16.4"
macos_version: macos-15
test-destination-os: "18.4"
platform: "iOS"
device: "iPhone 16 Pro"

- name: iOS 26
macos_version: macos-26
xcode: "26.1.1"
test-destination-os: "26.1"
platform: "iOS"
device: "iPhone 17 Pro"

- name: macOS 15
macos_version: macos-15
xcode: "16.4"
test-destination-os: "latest"
platform: "macOS"

- name: macOS 26
macos_version: macos-26
xcode: "26.1.1"
test-destination-os: "26.1"
platform: "macOS"

unit-tests-required-check:
needs:
[
ready-to-merge-gate,
files-changed,
spm-integrations-tests,
test-integration,
]
name: Swift Log Tests - Required Check
# 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
38 changes: 29 additions & 9 deletions .github/workflows/unit-test-common.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,17 @@ on:
default: false
type: boolean

working_directory:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Missing default for working_directory in unit-test-common.yml breaks existing callers and causes workflow failures.
Severity: CRITICAL | Confidence: High

🔍 Detailed Analysis

The unit-test-common.yml workflow's working_directory input is required: false but lacks a default: value. Existing callers, such as test.yml, do not provide this parameter. When working_directory is an empty string, critical steps like ls -la ${{ inputs.working_directory }} will fail with ls: cannot access '': No such file or directory. Additionally, the report_paths for JUnit reports will incorrectly resolve to /build/reports/junit.xml, causing the report action to fail.

💡 Suggested Fix

Add a default: value to the working_directory input in unit-test-common.yml or update all existing callers (e.g., test.yml) to provide this parameter.

🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: .github/workflows/unit-test-common.yml#L72

Potential issue: The `unit-test-common.yml` workflow's `working_directory` input is
`required: false` but lacks a `default:` value. Existing callers, such as `test.yml`, do
not provide this parameter. When `working_directory` is an empty string, critical steps
like `ls -la ${{ inputs.working_directory }}` will fail with `ls: cannot access '': No
such file or directory`. Additionally, the `report_paths` for JUnit reports will
incorrectly resolve to `/build/reports/junit.xml`, causing the report action to fail.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 5615901

description: "The working directory to run the tests in"
required: false
type: string

path_to_scripts_folder:
description: "The path to the scripts folder"
default: ./scripts
required: false
type: string

jobs:
unit-tests:
name: Unit ${{inputs.name}}
Expand All @@ -77,7 +88,9 @@ jobs:
if: ${{!inputs.should_skip}}
steps:
- uses: actions/checkout@v6
- run: ./scripts/ci-select-xcode.sh "$XCODE_VERSION"
- name: Select Xcode version
working-directory: ${{ inputs.working_directory }}
run: ${{ inputs.path_to_scripts_folder }}/ci-select-xcode.sh "$XCODE_VERSION"
env:
XCODE_VERSION: ${{inputs.xcode}}

Expand All @@ -86,42 +99,47 @@ jobs:

# Install platform runtimes that are not included by default
- name: Install required platforms
working-directory: ${{ inputs.working_directory }}
if: ${{ inputs.install_platforms }}
env:
PLATFORMS: ${{ inputs.platform }}
OS_VERSION: ${{ inputs.test-destination-os }}
run: ./scripts/ci-install-platforms.sh --platforms "$PLATFORMS" --os-version "$OS_VERSION"
run: ${{ inputs.path_to_scripts_folder }}/ci-install-platforms.sh --platforms "$PLATFORMS" --os-version "$OS_VERSION"

- name: Ensure required runtime is loaded
# Ideally we will not need this, but CI sometimes is failing to load some runtimes, this will ensure they are loaded
if: ${{ inputs.platform == 'iOS' }}
timeout-minutes: 5 # 5 minutes timeout
working-directory: ${{ inputs.working_directory }}
env:
OS_VERSION: ${{ inputs.test-destination-os }}
run: ./scripts/ci-ensure-runtime-loaded.sh --os-version "$OS_VERSION"
run: ${{ inputs.path_to_scripts_folder }}/ci-ensure-runtime-loaded.sh --os-version "$OS_VERSION"

# Create simulator devices for non-preinstalled simulators
# Required for iOS 16.4, iOS 17.5 (on Xcode 15.4), and iOS/tvOS 26.1
- name: Create simulator device
if: ${{ inputs.create_device }}
working-directory: ${{ inputs.working_directory }}
env:
PLATFORM: ${{ inputs.platform }}
OS_VERSION: ${{ inputs.test-destination-os }}
DEVICE_NAME: ${{inputs.device}}
run: ./scripts/ci-create-simulator.sh --platform "$PLATFORM" --os-version "$OS_VERSION" --device-name "$DEVICE_NAME"
run: ${{ inputs.path_to_scripts_folder }}/ci-create-simulator.sh --platform "$PLATFORM" --os-version "$OS_VERSION" --device-name "$DEVICE_NAME"

# Boot created simulators to ensure they're ready before tests run
# Based on CircleCI forum comment, booting is especially important for Xcode 26: https://discuss.circleci.com/t/xcode-26-rc/54066/18
- name: Boot simulator
if: ${{ inputs.create_device && inputs.platform == 'iOS' }}
working-directory: ${{ inputs.working_directory }}
env:
XCODE_VERSION: ${{ inputs.xcode }}
DEVICE_NAME: ${{ inputs.device }}
OS_VERSION: ${{ inputs.test-destination-os }}
run: ./scripts/ci-boot-simulator.sh --xcode "$XCODE_VERSION" --device "$DEVICE_NAME" --os-version "$OS_VERSION"
run: ${{ inputs.path_to_scripts_folder }}/ci-boot-simulator.sh --xcode "$XCODE_VERSION" --device "$DEVICE_NAME" --os-version "$OS_VERSION"

# We split building and running tests in two steps so we know how long running the tests takes.
- name: Build Tests
working-directory: ${{ inputs.working_directory }}
id: build_tests
env:
PLATFORM: ${{ inputs.platform }}
Comment on lines 147 to 165

This comment was marked as outdated.

Expand All @@ -130,7 +148,7 @@ jobs:
DEVICE_NAME: ${{ inputs.device }}
SCHEME: ${{ inputs.scheme }}
run: |
./scripts/sentry-xcodebuild.sh \
${{ inputs.path_to_scripts_folder }}/sentry-xcodebuild.sh \
--platform "$PLATFORM" \
--os "$OS_VERSION" \
--ref "$REF_NAME" \
Expand All @@ -146,14 +164,15 @@ jobs:
- name: Run Flaky Tests
# Only the Sentry Scheme has the Flaky TestPlan.
if: ${{ inputs.scheme == 'Sentry' }}
working-directory: ${{ inputs.working_directory }}
env:
PLATFORM: ${{ inputs.platform }}
OS_VERSION: ${{ inputs.test-destination-os }}
REF_NAME: ${{ github.ref_name }}
DEVICE_NAME: ${{ inputs.device }}
SCHEME: ${{ inputs.scheme }}
run: |
./scripts/sentry-xcodebuild.sh \
${{ inputs.path_to_scripts_folder }}/sentry-xcodebuild.sh \
--platform "$PLATFORM" \
--os "$OS_VERSION" \
--ref "$REF_NAME" \
Expand All @@ -169,14 +188,15 @@ jobs:
# passed to xcodebuild doesn't end up in the job name,
# because GitHub Actions don't provide an easy way of
# manipulating string in expressions.
working-directory: ${{ inputs.working_directory }}
env:
PLATFORM: ${{ inputs.platform }}
OS_VERSION: ${{ inputs.test-destination-os }}
REF_NAME: ${{ github.ref_name }}
DEVICE_NAME: ${{ inputs.device }}
SCHEME: ${{ inputs.scheme }}
run: |
./scripts/sentry-xcodebuild.sh \
${{ inputs.path_to_scripts_folder }}/sentry-xcodebuild.sh \
--platform "$PLATFORM" \
--os "$OS_VERSION" \
--ref "$REF_NAME" \
Expand All @@ -190,7 +210,7 @@ jobs:
uses: mikepenz/action-junit-report@e08919a3b1fb83a78393dfb775a9c37f17d8eea6 # v6.0.1
if: always()
with:
report_paths: "build/reports/junit.xml"
report_paths: "${{ inputs.working_directory }}/build/reports/junit.xml"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Empty working_directory creates invalid absolute path prefix

The report_paths value "${{ inputs.working_directory }}/build/reports/junit.xml" will produce /build/reports/junit.xml when working_directory is not provided (defaults to empty string). This changes a relative path to an absolute path starting with /, breaking existing callers of unit-test-common.yml that don't specify working_directory (like fast-pr-checks.yml and test.yml). The previous value was simply "build/reports/junit.xml" without the prefix.

Fix in Cursor Fix in Web

fail_on_failure: true
fail_on_parse_error: true
detailed_summary: true
Expand Down
12 changes: 12 additions & 0 deletions scripts/prepare-package.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ Options:
--change-path true|false Whether to swap SPM binary URLs for local paths (default: false)
--remove-binary-targets true|false
Whether to keep only SentryDistribution product/target (default: false)
--update-path-to-sentry-cocoa true|false
Whether to update the path to the sentry-cocoa directory (default: false)
-h, --help Show this help message
USAGE
}
Expand All @@ -30,6 +32,7 @@ IS_PR="false"
REMOVE_DUPLICATE="false"
CHANGE_PATH="false"
REMOVE_BINARY_TARGETS="false"
UPDATE_PATH_TO_SENTRY_COCOA="false"

while [[ $# -gt 0 ]]; do
case "$1" in
Expand Down Expand Up @@ -58,6 +61,11 @@ while [[ $# -gt 0 ]]; do
REMOVE_BINARY_TARGETS="$2"
shift 2
;;
--update-path-to-sentry-cocoa)
[[ $# -lt 2 ]] && { echo "Missing value for $1" >&2; exit 1; }
UPDATE_PATH_TO_SENTRY_COCOA="$2"
shift 2
;;
-h|--help)
usage
exit 0
Expand Down Expand Up @@ -131,6 +139,10 @@ var targets: [Target] = [\
sed -i '' '/^let env = getenv("EXPERIMENTAL_SPM_BUILDS")/,/^}/d' "$PACKAGE_FILE"
fi

if is_enabled "$UPDATE_PATH_TO_SENTRY_COCOA"; then
sed -i '' 's|\.package(url: "https://github\.com/getsentry/sentry-cocoa", from: "[^"]*")|.package(path: "'../../../'")|g' "$PACKAGE_FILE"

This comment was marked as outdated.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Sed produces invalid Swift syntax with literal quotes

The sed command at line 143 produces invalid Swift package path syntax. The replacement pattern '.package(path: "'../../../'")' will generate .package(path: "'../../../'") in the Package.swift file, where the path string contains literal single quote characters. Swift package paths require just the path without extra quotes, like .package(path: "../../../"). The literal single quotes will cause the package resolution to fail because it will look for a directory named '../../../' instead of ../../../.

Fix in Cursor Fix in Web

fi
Comment on lines 142 to 144

This comment was marked as outdated.


echo
echo "===== $PACKAGE_FILE (after prepare-package.sh) ====="
cat "$PACKAGE_FILE"
Expand Down
31 changes: 20 additions & 11 deletions scripts/sentry-xcodebuild.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ DERIVED_DATA_PATH=""
TEST_SCHEME="Sentry"
TEST_PLAN=""
RESULT_BUNDLE_PATH="results.xcresult"
SPM_PROJECT="false"

usage() {
echo "Usage: $0"
Expand All @@ -36,6 +37,7 @@ usage() {
echo " -s|--scheme <scheme> Test scheme (default: Sentry)"
echo " -t|--test-plan <plan> Test plan name (default: empty)"
echo " -R|--result-bundle <path> Result bundle path (default: results.xcresult)"
echo " -S|--spm-project <bool> Use SPM project (default: false)"
exit 1
}

Expand Down Expand Up @@ -82,6 +84,10 @@ while [[ $# -gt 0 ]]; do
RESULT_BUNDLE_PATH="$2"
shift 2
;;
-S|--spm-project)
SPM_PROJECT="true"
shift 1
;;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Argument parsing mismatch for spm-project flag

The --spm-project option's usage documentation (line 40) indicates it accepts a <bool> argument, but the implementation unconditionally sets SPM_PROJECT="true" and only does shift 1 instead of shift 2. All other options in this script read $2 and shift by 2. If a user runs --spm-project false following the documented interface, the variable is incorrectly set to "true" and then "false" is left as the next argument, causing an "Unknown option" error.

Fix in Cursor Fix in Web

*)
echo "Unknown option: $1"
usage
Expand Down Expand Up @@ -169,16 +175,22 @@ if [ -n "$TEST_PLAN" ]; then
TEST_PLAN_ARGS+=("-testPlan" "$TEST_PLAN")
fi

# Build xcodebuild arguments based on project type
XCODEBUILD_ARGS=()
if [ $SPM_PROJECT != "true" ]; then
XCODEBUILD_ARGS+=("-workspace" "Sentry.xcworkspace")
XCODEBUILD_ARGS+=("-configuration" "$CONFIGURATION")
fi
XCODEBUILD_ARGS+=("-scheme" "$TEST_SCHEME")
XCODEBUILD_ARGS+=("${TEST_PLAN_ARGS[@]+${TEST_PLAN_ARGS[@]}}")
XCODEBUILD_ARGS+=("-destination" "$DESTINATION")

if [ $RUN_BUILD_FOR_TESTING == true ]; then
# When no test plan is provided, we skip the -testPlan argument so xcodebuild uses the default test plan
log_notice "Running xcodebuild build-for-testing"

set -o pipefail && NSUnbufferedIO=YES xcodebuild \
-workspace Sentry.xcworkspace \
-scheme "$TEST_SCHEME" \
"${TEST_PLAN_ARGS[@]+${TEST_PLAN_ARGS[@]}}" \
-configuration "$CONFIGURATION" \
-destination "$DESTINATION" \
"${XCODEBUILD_ARGS[@]}" \
build-for-testing 2>&1 |
tee raw-build-for-testing-output.log |
xcbeautify --preserve-unbeautified
Expand All @@ -188,13 +200,10 @@ if [ $RUN_TEST_WITHOUT_BUILDING == true ]; then
# When no test plan is provided, we skip the -testPlan argument so xcodebuild uses the default test plan
log_notice "Running xcodebuild test-without-building"

XCODEBUILD_ARGS+=("-resultBundlePath" "$RESULT_BUNDLE_PATH")

set -o pipefail && NSUnbufferedIO=YES xcodebuild \
-workspace Sentry.xcworkspace \
-scheme "$TEST_SCHEME" \
"${TEST_PLAN_ARGS[@]+${TEST_PLAN_ARGS[@]}}" \
-configuration "$CONFIGURATION" \
-destination "$DESTINATION" \
-resultBundlePath "$RESULT_BUNDLE_PATH" \
"${XCODEBUILD_ARGS[@]}" \
test-without-building 2>&1 |
tee raw-test-output.log |
xcbeautify --report junit
Expand Down
Loading