Skip to content

Remove experimental spi from issue severity#1247

Merged
suzannaratcliff merged 9 commits into
swiftlang:mainfrom
suzannaratcliff:suzannaratcliff/issue-severity
Aug 5, 2025
Merged

Remove experimental spi from issue severity#1247
suzannaratcliff merged 9 commits into
swiftlang:mainfrom
suzannaratcliff:suzannaratcliff/issue-severity

Conversation

@suzannaratcliff

@suzannaratcliff suzannaratcliff commented Jul 30, 2025

Copy link
Copy Markdown
Contributor

Remove experimental spi from issue severity

Motivation:

The Issue Severity proposal was approved.

https://github.com/swiftlang/swift-evolution/blob/main/proposals/testing/0013-issue-severity-warning.md

Modifications:

Removed experimental spi from issue severity and updated comments.

Checklist:

  • Code and documentation should follow the style of the Style Guide.
  • If public symbols are renamed or modified, DocC references should be updated.

@stmontgomery stmontgomery added documentation 📚 Improvements or additions to documentation enhancement New feature or request public-api Affects public API issue-handling Related to Issue handling within the testing library labels Jul 30, 2025
@stmontgomery stmontgomery added this to the Swift 6.x (main) milestone Jul 30, 2025
@stmontgomery

stmontgomery commented Jul 30, 2025

Copy link
Copy Markdown
Contributor

@swift-ci please test

stmontgomery
stmontgomery reviewed Jul 30, 2025
View reviewed changes
Comment thread Sources/Testing/Issues/Issue+Recording.swift Outdated
stmontgomery
stmontgomery reviewed Jul 30, 2025
View reviewed changes
Comment thread Sources/Testing/Issues/Issue+Recording.swift Outdated
grynspan
grynspan reviewed Jul 30, 2025
View reviewed changes
Comment thread Sources/Testing/Issues/Issue+Recording.swift Outdated
@stmontgomery

stmontgomery commented Jul 30, 2025

Copy link
Copy Markdown
Contributor

To enable the posting of issues with warning severity to event handlers, we need to also change Configuration.EventHandlingOptions.isWarningIssueRecordedEventEnabled to be true by default

@stmontgomery stmontgomery requested a review from iamleeg July 30, 2025 17:29
@grynspan

grynspan commented Jul 30, 2025

Copy link
Copy Markdown
Contributor

To enable the posting of issues with warning severity to event handlers, we need to also change Configuration.EventHandlingOptions.isWarningIssueRecordedEventEnabled to be true by default

But only for v6_3 onward and presumably the Xcode 16 legacy path, right? v0 can't handle them.

stmontgomery
stmontgomery reviewed Jul 30, 2025
View reviewed changes
Comment thread Sources/Testing/Issues/Issue.swift
@stmontgomery

stmontgomery commented Jul 30, 2025

Copy link
Copy Markdown
Contributor

To enable the posting of issues with warning severity to event handlers, we need to also change Configuration.EventHandlingOptions.isWarningIssueRecordedEventEnabled to be true by default.

But only for v6_3 onward and presumably the Xcode 16 legacy path, right? v0 can't handle them.

Compatibility with older event stream versions is handled by this entry point logic:

swift-testing/Sources/Testing/ABI/EntryPoints/EntryPoint.swift

Lines 625 to 628 in f0047e6

case .some(..<ABI.v6_3.versionNumber):
// If the event stream version was explicitly specified to a value < 6.3,
// disable the warning issue event to maintain legacy behavior.
configuration.eventHandlingOptions.isWarningIssueRecordedEventEnabled = false

@suzannaratcliff
/// @metadata {
0518549 
  ///   @available(Swift, introduced: 6.3)
  /// }
@suzannaratcliff

suzannaratcliff commented Jul 30, 2025

Copy link
Copy Markdown
Contributor Author

@stmontgomery I think that Configuration.EventHandlingOptions.isWarningIssueRecordedEventEnabled is already set to true

@stmontgomery

stmontgomery commented Jul 31, 2025

Copy link
Copy Markdown
Contributor

@stmontgomery I think that Configuration.EventHandlingOptions.isWarningIssueRecordedEventEnabled is already set to true

I don't think it is; maybe we're talking about different things. I'm referring to this line in Configuration.swift:

swift-testing/Sources/Testing/Running/Configuration.swift

Line 193 in f0047e6

public var isWarningIssueRecordedEventEnabled: Bool = false

iamleeg
iamleeg requested changes Jul 31, 2025
View reviewed changes

@iamleeg iamleeg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Small but important issues in the documentation.

Comment thread Sources/Testing/Issues/Issue+Recording.swift Outdated
Comment thread Sources/Testing/Issues/Issue+Recording.swift Outdated
@suzannaratcliff suzannaratcliff requested a review from iamleeg August 4, 2025 16:26
@suzannaratcliff suzannaratcliff force-pushed the suzannaratcliff/issue-severity branch from 10ddbe1 to bf005b0 Compare August 5, 2025 16:13
stmontgomery
stmontgomery reviewed Aug 5, 2025
View reviewed changes
Comment thread Sources/Testing/Issues/Issue+Recording.swift Outdated
stmontgomery
stmontgomery reviewed Aug 5, 2025
View reviewed changes
Comment thread Sources/Testing/Issues/Issue+Recording.swift
stmontgomery
stmontgomery reviewed Aug 5, 2025
View reviewed changes
Comment thread Sources/Testing/Issues/Issue+Recording.swift
grynspan
grynspan reviewed Aug 5, 2025
View reviewed changes
Comment thread Sources/Testing/Running/Configuration.swift
@suzannaratcliff suzannaratcliff merged commit 64856c7 into swiftlang:main Aug 5, 2025
@suzannaratcliff suzannaratcliff mentioned this pull request Aug 5, 2025
Merged
2 tasks
suzannaratcliff added a commit that referenced this pull request Aug 5, 2025
@suzannaratcliff
Fix test: issueCountSummingAtRunEnd which was broken by #1247 (#1250)
c60f130 
Fix test: issueCountSummingAtRunEnd which was broken by
#1247

Enabling warnings by default broke the test: issueCountSummingAtRunEnd.
This fixes that.

### Modifications:

I updated the expected output to contain warnings.

### Checklist:

- [x] Code and documentation should follow the style of the [Style
Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md).
- [x] If public symbols are renamed or modified, DocC references should
be updated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stmontgomery stmontgomery stmontgomery approved these changes

@briancroom briancroom Awaiting requested review from briancroom briancroom is a code owner

@iamleeg iamleeg Awaiting requested review from iamleeg

@grynspan grynspan Awaiting requested review from grynspan grynspan is a code owner

Assignees

@suzannaratcliff suzannaratcliff

Labels

documentation 📚 Improvements or additions to documentation enhancement New feature or request issue-handling Related to Issue handling within the testing library public-api Affects public API

Projects

None yet

Milestone

Swift 6.3.0

Development

Successfully merging this pull request may close these issues.

4 participants

@suzannaratcliff @stmontgomery @grynspan @iamleeg