Skip to content

Fix benchmark succeeding – even though Benchmark build failed#3502

Merged
fabianfett merged 12 commits intomainfrom
ff-fix-benchmark-non-failure
Feb 17, 2026
Merged

Fix benchmark succeeding – even though Benchmark build failed#3502
fabianfett merged 12 commits intomainfrom
ff-fix-benchmark-non-failure

Conversation

@fabianfett
Copy link
Copy Markdown
Member

@fabianfett fabianfett commented Feb 5, 2026

Motivation:

Currently if there is an error in the Benchmark targets, that prevents the Benchmark to build correctly, we still get a CI success.

Modifications:

  • Check for the threshold changed error

Result:

  • More reliable CI

@fabianfett fabianfett added the semver/none No version bump required. label Feb 5, 2026
@fabianfett
Copy link
Copy Markdown
Member Author

fabianfett commented Feb 5, 2026

This run shows the bad behavior:
https://github.com/apple/swift-nio/actions/runs/21718585153/job/62641524910?pr=3502

[13/15] Compiling NIOCoreBenchmarks Benchmarks.swift
/swift-nio/Benchmarks/Benchmarks/NIOCoreBenchmarks/Benchmarks.swift:40:1: error: cannot find 'asdasd' in scope
 38 | 
 39 | let benchmarks = {
 40 | asdasd
    | `- error: cannot find 'asdasd' in scope
 41 |     #if LOCAL_TESTING
 42 |     let defaultMetrics: [BenchmarkMetric] = [

Benchmark failed to run due to build error.
** Recalculating thresholds...
[0/8] Compiling CSystem shims.c
[1/8] Write swift-version-24593BA9C3E375BF.txt
[2/8] Wrapping AST for ArgumentParserToolInfo for debugging

glbrntt
glbrntt requested changes Feb 6, 2026
View reviewed changes
Benchmarks/Benchmarks/NIOCoreBenchmarks/Benchmarks.swift Outdated
// MARK: - Benchmarks

let benchmarks = {
asdasd
Copy link
Copy Markdown
Contributor

@glbrntt glbrntt Feb 6, 2026

Choose a reason for hiding this comment

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

You should probably remove this bit now 😅

Copy link
Copy Markdown
Member Author

@fabianfett fabianfett Feb 11, 2026

Choose a reason for hiding this comment

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

Obviously reverted now

@fabianfett
Copy link
Copy Markdown
Member Author

fabianfett commented Feb 11, 2026

CI run, that proves that this change does, what it shall do:
https://github.com/apple/swift-nio/actions/runs/21906654812/job/63248630543

@fabianfett fabianfett requested a review from glbrntt February 11, 2026 13:31
@fabianfett fabianfett enabled auto-merge (squash) February 11, 2026 13:34
simonjbeaumont
simonjbeaumont requested changes Feb 11, 2026
View reviewed changes
simonjbeaumont
simonjbeaumont approved these changes Feb 11, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@simonjbeaumont simonjbeaumont left a comment

Choose a reason for hiding this comment

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

Nice one!

@fabianfett fabianfett merged commit 2fdda6c into main Feb 17, 2026
55 checks passed
@fabianfett fabianfett deleted the ff-fix-benchmark-non-failure branch February 17, 2026 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@simonjbeaumont simonjbeaumont simonjbeaumont approved these changes

@glbrntt glbrntt glbrntt approved these changes

Assignees

No one assigned

Labels

semver/none No version bump required.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@fabianfett @simonjbeaumont @glbrntt