-
Notifications
You must be signed in to change notification settings - Fork 2.4k
[DRAFT] [ST-XXXX] Test Issue Severity #2794
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.
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
Open
suzannaratcliff
wants to merge
14
commits into
swiftlang:main
Choose a base branch
from
suzannaratcliff:suzannaratcliff/issue-severity-warnings
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+212
−0
Open
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
1ca81dc
Test Issue Warnings
suzannaratcliff a1fcbcb
Clean up format
suzannaratcliff 6f26d42
Add example for isFailure and severity
suzannaratcliff 65ba883
Add Acknowledgments
suzannaratcliff e649f9c
Add use cases
suzannaratcliff d51c8e8
Updates based on comments and future direction
suzannaratcliff a9cff4f
Add pitch link
suzannaratcliff 4d70fc3
Add another alternative approach
suzannaratcliff 98825de
Update Alternatives considered
suzannaratcliff baba8c7
Update format
suzannaratcliff c50f733
update comment
suzannaratcliff 46f5a20
Fix spacing and definitions
suzannaratcliff ae3640f
Update Integration with supporting tools section
suzannaratcliff 3adec7d
Add console output and details to try this out
suzannaratcliff File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next
Next commit
Test Issue Warnings
- Loading branch information
commit 1ca81dcb3e50efe1af9c89768c07dc29c05c1309
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
# [DRAFT] Test Issue Warnings | ||
|
||
* Proposal: [ST-XXXX](XXXX-issue-severity-warning.md) | ||
* Authors: [Suzy Ratcliff](https://github.com/suzannaratcliff) | ||
* Review Manager: TBD | ||
* Status: **Pitched** | ||
* Bug: [swiftlang/swift-testing#1075](https://github.com/swiftlang/swift-testing/pull/1075) | ||
* Implementation: [swiftlang/swift-testing#1075](https://github.com/swiftlang/swift-testing/pull/1075) | ||
|
||
suzannaratcliff marked this conversation as resolved.
Show resolved
Hide resolved
|
||
## Introduction | ||
|
||
I propose introducing a new API to Swift Testing that allows developers to record issues with a specified severity level. By default, all issues will have severity level “error”, and a new “warning” level will be added to represent less severe issues. The effects of the warning recorded on a test will not cause a failure but will be included in the test results for inspection after the run is complete. | ||
|
||
## Motivation | ||
|
||
Currently, when an issue arises during a test, the only possible outcome is to mark the test as failed. This presents a challenge for users who want a deeper insight into the events occurring within their tests. By introducing a dedicated mechanism to record issues that do not cause test failure, users can more effectively inspect and diagnose problems at runtime and review results afterward. This enhancement provides greater flexibility and clarity in test reporting, ultimately improving the debugging and analysis process. | ||
|
||
## Proposed solution | ||
We propose introducing a new property on Issues in Swift Testing called `Severity`, that represents if an issue is a warning or an error. | ||
The default Issue severity will still be error and users can create set the severity when they record an issue. | ||
|
||
Test authors will be able to inspect if the issue is a failing issue and will be able to check the severity. | ||
|
||
## Detailed design | ||
|
||
*Severity Enum* | ||
|
||
We introduce a Severity enum to categorize issues detected during testing. This enum is crucial for distinguishing between different levels of test issues and is defined as follows: | ||
|
||
The `Severity` enum is defined as follows: | ||
|
||
```swift | ||
public enum Severity: Sendable { | ||
suzannaratcliff marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/// The severity level for an issue which should be noted but is not | ||
/// necessarily an error. | ||
/// | ||
/// An issue with warning severity does not cause the test it's associated | ||
/// with to be marked as a failure, but is noted in the results. | ||
case warning | ||
|
||
/// The severity level for an issue which represents an error in a test. | ||
/// | ||
/// An issue with error severity causes the test it's associated with to be | ||
/// marked as a failure. | ||
case error | ||
} | ||
``` | ||
|
||
*Recording Non-Failing Issues* | ||
To enable test authors to log non-failing issues without affecting test results, we provide a method for recording such issues: | ||
|
||
```swift | ||
Issue.record("My comment", severity: .warning) | ||
``` | ||
|
||
*Issue Type Enhancements* | ||
The Issue type is enhanced with two new properties to better handle and report issues: | ||
- `severity`: This property allows access to the specific severity level of an issue, enabling more precise handling of test results. | ||
|
||
```swift | ||
/// The severity of the issue. | ||
public var severity: Severity | ||
|
||
``` | ||
- `isFailure`: A boolean property to determine if an issue results in a test failure, thereby helping in result aggregation and reporting. | ||
```swift | ||
|
||
/// Whether or not this issue should cause the test it's associated with to be | ||
/// considered a failure. | ||
/// | ||
/// The value of this property is `true` for issues which have a severity level of | ||
/// ``Issue/Severity/error`` or greater and are not known issues via | ||
/// ``withKnownIssue(_:isIntermittent:sourceLocation:_:when:matching:)``. | ||
/// Otherwise, the value of this property is `false.` | ||
/// | ||
/// Use this property to determine if an issue should be considered a failure, instead of | ||
/// directly comparing the value of the ``severity`` property. | ||
public var isFailure: Bool | ||
``` | ||
|
||
For more details, refer to the [Issue Documentation](https://developer.apple.com/documentation/testing/issue). | ||
|
||
This revision aims to clarify the functionality and usage of the `Severity` enum and `Issue` properties while maintaining consistency with the existing Swift API standards. | ||
|
||
## Alternatives considered | ||
|
||
- Doing Nothing: Although there have been recurring requests over the years to support non-failing issues, we did not have a comprehensive solution until now. This year, we finally had the necessary components to implement this feature effectively. | ||
|
||
- Separate Issue Creation and Recording: We considered providing a mechanism to create issues independently before recording them, rather than passing the issue details directly to the `record` method. This approach was ultimately set aside in favor of simplicity and directness in usage. | ||
|
||
- Naming of `isFailure` vs. `isFailing`: We evaluated whether to name the property `isFailing` instead of `isFailure`. The decision to use `isFailure` was made to adhere to naming conventions and ensure clarity and consistency within the API. | ||
|
||
- Severity-Only Checking: We deliberated not exposing `isFailure` and relying solely on `severity` checks. However, this was rejected because it would require test authors to overhaul their code should we introduce additional severity levels in the future. By providing `isFailure`, we offer a straightforward way to determine test outcome impact, complementing the severity feature. | ||
|
||
## Acknowledgments | ||
|
||
Thanks to Stuart Montgomery for creating and implementing severity in Swift Testing. | ||
|
||
Thanks to Brian Croom and Jonathan Grynspan for feedback on warnings along the way. |
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.
Uh oh!
There was an error while loading. Please reload this page.