Introduce a severity level for issues, and a 'warning' severity #931
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
stmontgomery added enhancement 
stmontgomery commented Jan 28, 2025
grynspan reviewed Jan 28, 2025
grynspan reviewed Jan 28, 2025
grynspan reviewed Jan 28, 2025
grynspan reviewed Jan 28, 2025
grynspan reviewed Jan 28, 2025
Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift Outdated Show resolved Hide resolved
grynspan reviewed Jan 28, 2025
grynspan reviewed Jan 28, 2025
grynspan reviewed Jan 28, 2025
Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift Outdated Show resolved Hide resolved
grynspan reviewed Jan 28, 2025
grynspan reviewed Jan 28, 2025
Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift Outdated Show resolved Hide resolved
| @swift-ci please test |
grynspan reviewed Jan 30, 2025
grynspan reviewed Jan 30, 2025
grynspan reviewed Jan 30, 2025
grynspan reviewed Jan 30, 2025
grynspan reviewed Jan 30, 2025
grynspan reviewed Jan 30, 2025
| @swift-ci please test |
stmontgomery force-pushed the issue-severity branch from fc0b7bb to b01c820 Compare | @swift-ci please test |
grynspan reviewed Feb 10, 2025
grynspan reviewed Feb 10, 2025
grynspan reviewed Feb 10, 2025
2 tasks
| @swift-ci please test macOS |
grynspan reviewed Feb 11, 2025
grynspan reviewed Feb 11, 2025
grynspan approved these changes Feb 11, 2025
| @swift-ci please test |
| It seems like this PR uncovered a compiler crash error: swiftlang/swift#79304 I don't think it is this PR's fault but it just uncovered a latent bug in the compiler. The crashes are seen on the CIs: I'm not sure what's the best way forward, but I wonder if we could work around like editing |
stmontgomery added a commit to stmontgomery/swift-testing that referenced this pull request Feb 12, 2025
…ty (swiftlang#931)" This reverts commit 6a49142.
glessard added a commit that referenced this pull request Feb 12, 2025
Revert "Introduce a severity level for issues, and a 'warning' severity (#931)"
2 tasks
stmontgomery added a commit to stmontgomery/swift-testing that referenced this pull request Feb 12, 2025
…evert swiftlang#931) Revert "Merge pull request swiftlang#950 from stmontgomery/revert-issue-severity" (swiftlang#950) This reverts commit 9998633, reversing changes made to 55d0023.
2 tasks
stmontgomery added a commit that referenced this pull request Feb 12, 2025
…evert #931) (#952) This un-reverts #950, effectively reintroducing the changes recently landed in #931. The revert was needed because it revealed a latent bug in the Swift compiler, tracked by swiftlang/swift#79304. I reproduced that failure and included a workaround in the second commit on this PR. ### 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.
stmontgomery added a commit that referenced this pull request Feb 12, 2025
…#951) This modifies `Package.swift` to enable Library Evolution for builds of the package. ### Motivation: I recently landed a change (#931) which passed our project-level CI but later failed in Swift CI. The difference ended up being due to the latter building with Library Evolution (LE) enabled, whereas our project-level CI builds via SwiftPM and does not enable LE. The change was reverted (#950) but this revealed a gap in our testing strategy. We should always build these targets with LE enabled. ### 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. Fixes rdar://144655439
2 tasks
stmontgomery added a commit that referenced this pull request Feb 28, 2025
This updates `Event.JUnitXMLRecorder` to ignore `Issue` instances whose `severity` is less than `.error` (such as `.warning`). ### Motivation: The concept of issue severity was recently added in #931 (but was reverted and re-landed in #952), and that did not adjust the JUnit XML recorder logic. The JUnit XML schema we currently attempt to adhere to does not appear to have a way to represent non-fatal issues, so I think it would be best for now to ignore these issues. ### Modifications: - Implement the fix and a validating unit test. - (Drive-by) Fix a nearby test I noticed wasn't actually working as intended and wasn't properly validating the fix it was intended to. ### 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.
3 tasks
suzannaratcliff added a commit that referenced this pull request Apr 10, 2025
Introduce a severity level when recording issues ### Motivation: In order to create issues that don't fail a test this introduces a parameter to specify the severity of the issue. This is in support of work added here for an issue severity: #931 This is experimental. Example usage: `Issue.record("My comment", severity: .warning)` ### Modifications: I modified the `Issue.record` method signature to take in a severity level so that users can create issues that are not failing. ### 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. - [x] Add tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
This introduces the concept of severity to the
Issuetype, represented by a new enumIssue.Severitywith two cases:.warningand.error. Error is the default severity for all issues, matching current behavior, but warning is provided as a new option which does not cause the test the issue is associated with to be marked as a failure.In this PR, these are SPI but they could be considered for promotion to public API eventually. Additional work would be needed to permit test authors to record issues with severity <
.error, since APIs likeIssue.record()are not being modified at this time to allow customizing the severity.Motivation:
There are certain situations where a problem may arise during a test that doesn't necessarily affect its outcome or signal an important problem, but is worth calling attention to. A specific example use case I have in mind is to allow the testing library to record a warning issue about problems with the arguments passed to a parameterized test, such as having duplicate arguments.
Modifications:
Issue.Severityas an SPI enum.severitytoIssuewith default value.error.EXIT_SUCCESSif all issues recorded had severity <.error.Checklist: