Skip to content

Conversation

stmontgomery
Copy link
Contributor

@stmontgomery stmontgomery commented Jan 27, 2025

This introduces the concept of severity to the Issue type, represented by a new enum Issue.Severity with two cases: .warning and .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 like Issue.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:

  • Introduce Issue.Severity as an SPI enum.
  • Introduce an SPI property severity to Issue with default value .error.
  • Modify entry point logic to exit with EXIT_SUCCESS if all issues recorded had severity < .error.
  • Modify console output formatting logic and data structures to represent warning issues sensibly.

Checklist:

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

@stmontgomery stmontgomery added enhancement New feature or request tools integration 🛠️ Integration of swift-testing into tools/IDEs issue-handling Related to Issue handling within the testing library labels Jan 27, 2025
@stmontgomery stmontgomery added this to the Swift 6.x milestone Jan 27, 2025
@stmontgomery stmontgomery self-assigned this Jan 27, 2025
@stmontgomery
Copy link
Contributor Author

@swift-ci please test

@stmontgomery
Copy link
Contributor Author

@swift-ci please test

@stmontgomery
Copy link
Contributor Author

@swift-ci please test

@stmontgomery
Copy link
Contributor Author

@swift-ci please test macOS

Copy link
Contributor

@grynspan grynspan left a comment

Choose a reason for hiding this comment

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

A couple of test tweaks, but otherwise :shipit:

@stmontgomery
Copy link
Contributor Author

@swift-ci please test

@stmontgomery stmontgomery merged commit 6a49142 into swiftlang:main Feb 11, 2025
3 checks passed
@stmontgomery stmontgomery deleted the issue-severity branch February 11, 2025 17:04
@hjyamauchi
Copy link
Contributor

hjyamauchi commented Feb 11, 2025

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:
https://ci.swift.org/job/oss-swift-incremental-RA-macos-apple-silicon/8156/consoleText
https://ci-external.swift.org/job/swift-main-windows-toolchain/1055/consoleText

I'm not sure what's the best way forward, but I wonder if we could work around like editing issueCounts code somehow?

stmontgomery added a commit to stmontgomery/swift-testing that referenced this pull request Feb 12, 2025
glessard added a commit that referenced this pull request Feb 12, 2025
Revert "Introduce a severity level for issues, and a 'warning' severity (#931)"
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.
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
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.
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
Labels
enhancement New feature or request issue-handling Related to Issue handling within the testing library tools integration 🛠️ Integration of swift-testing into tools/IDEs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants