Skip to content

Remove experimental spi from issue severity #1247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 9 commits into from
Aug 5, 2025
Merged

Remove experimental spi from issue severity #1247

merged 9 commits into from
Aug 5, 2025

Conversation

suzannaratcliff
Copy link
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
Copy link
Contributor

@swift-ci please test

stmontgomery
stmontgomery reviewed Jul 30, 2025
View reviewed changes
stmontgomery
stmontgomery reviewed Jul 30, 2025
View reviewed changes
grynspan
grynspan reviewed Jul 30, 2025
View reviewed changes
@stmontgomery
Copy link
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
Copy link
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
@stmontgomery
Copy link
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
Copy link
Contributor Author

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

@stmontgomery
Copy link
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
Copy link
Contributor

@iamleeg iamleeg left a comment

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.

@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
stmontgomery
stmontgomery reviewed Aug 5, 2025
View reviewed changes
stmontgomery
stmontgomery reviewed Aug 5, 2025
View reviewed changes
grynspan
grynspan reviewed Aug 5, 2025
View reviewed changes
@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.x (main)
Development

Successfully merging this pull request may close these issues.
4 participants
@suzannaratcliff @stmontgomery @grynspan @iamleeg