-
Notifications
You must be signed in to change notification settings - Fork 2.5k
SwiftTesting: Include metadata for tags, bug, and time limit traits in event stream #3040
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
bkhouri
wants to merge
1
commit into
swiftlang:main
Choose a base branch
from
bkhouri:t/main/augment_event_stream_json
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.
Open
Changes from all commits
Commits
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
There are no files selected for viewing
207 changes: 207 additions & 0 deletions
207
proposals/testing/NNNN-include-tags-bugs-and-timeline-in-event-stream.md
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,207 @@ | ||
| # Include metadata for tags, bugs, and time limit traits in event stream | ||
|
|
||
| * Proposal: [ST-NNNN](NNNN-augment-event-json-abi.md) | ||
| * Authors: [Sam Khouri](https://github.com/bkhouri), | ||
| * Review Manager: TBD | ||
| * Status: **Awaiting review** | ||
| * Implementation: [swiftlang/swift-testing#1429](https://github.com/swiftlang/swift-testing/pull/1429) | ||
| * Review: [pitch](https://forums.swift.org/t/adding-additional-information-to-the-abi-json/83426) | ||
|
|
||
| ## Introduction | ||
|
|
||
| This proposal enhances Swift Testing's event JSON ABI by exposing test | ||
| metadata that is currently unavailable to external tools. By including test | ||
| tags, bug associations, and time limits in the JSON output, this allows third-party | ||
| tools to provide richer insights and more sophisticated test management capabilities. | ||
|
|
||
| ## Motivation | ||
|
|
||
| Swift Testing's event JSON stream provides data for external tooling, | ||
| enabling developers to build test analysis and reporting tools. | ||
| However, the current implementation lacks access to some test metadata that | ||
| developers may want to use to organize and manage their test suites. | ||
|
|
||
| Currently missing from the JSON output are: | ||
| - **Test tags**: Used for categorization | ||
| - **Bug associations**: Critical for tracking which tests verify specific bug fixes | ||
| - **Time limits**: Essential for performance monitoring and timeout management | ||
|
|
||
| This missing metadata limits the capabilities of external tools. For example: | ||
| - IDE extensions cannot provide tag-based test filtering | ||
| - CI/CD systems cannot generate reports grouped by test categories | ||
| - Performance monitoring tools cannot track tests with specific time constraints | ||
| - Bug tracking integrations cannot correlate test failures with known issues | ||
|
|
||
| By exposing this information, we unlock new possibilities for Swift Testing | ||
| tooling ecosystem. | ||
|
|
||
| ## Proposed solution | ||
|
|
||
| We propose enriching the test payload in the event JSON stream by adding three | ||
| metadata fields: | ||
|
|
||
| - **`tags`**: An array of strings where each item represents a single tag applied to the test, | ||
| enabling categorization and filtering | ||
| - **`bugs`**: An array of bug references, providing traceability between tests | ||
| and issue tracking | ||
| - **`timeLimit`**: The test's time limit in seconds, enabling performance monitoring | ||
| and timeout analysis | ||
|
|
||
| These additions leverage existing internal data structures, ensuring minimal performance | ||
| impact while maximizing the value delivered to external tools. | ||
|
|
||
| ## Detailed design | ||
|
|
||
| This enhancement builds upon the existing test metadata infrastructure already used | ||
| internally by Swift Testing. The implementation reuses established data structures, | ||
| ensuring consistency and minimizing complexity. | ||
|
|
||
| ### Implementation Strategy | ||
|
|
||
| Fields are only included when the test actually has at least one matching trait applied, preserving | ||
| backwards compatibility with previous versions. | ||
|
|
||
| ### JSON Schema Changes | ||
|
|
||
| The **Modified Backus-Naur Form (BNF)** delta would be: | ||
|
|
||
| ```diff | ||
| diff --git a/Documentation/ABI/JSON.md b/Documentation/ABI/JSON.md | ||
| index e4ff24a4..8d7d67ee 100644 | ||
| --- a/Documentation/ABI/JSON.md | ||
| +++ b/Documentation/ABI/JSON.md | ||
| @@ -157,10 +157,21 @@ additional `"testCases"` field describing the individual test cases. | ||
| ["displayName": <string>,] ; the user-supplied custom display name | ||
| "sourceLocation": <source-location>, ; where the test is defined | ||
| "id": <test-id>, | ||
| - "isParameterized": <bool> ; is this a parameterized test function or not? | ||
| + "isParameterized": <bool>, ; is this a parameterized test function or not? | ||
| + ["tags": <array:tag>,] ; the tags associated with this test function | ||
| + ["bugs": <array:bug>,] ; the bugs associated with this test function | ||
| + ["timeLimit": <number>] ; the time limit associated with this test function | ||
| } | ||
|
|
||
| <test-id> ::= <string> ; an opaque string representing the test case | ||
| + | ||
| +<tag> ::= "." <string> ; a string representation of a tag | ||
| + | ||
| +<bug> ::= { | ||
| + ["url": <string>,] ; the bug URL | ||
| + ["id": <string>,] ; the bug id | ||
| + ["title": <string>] ; the human readable bug title | ||
| +} ; | ||
| ``` | ||
|
|
||
| ### Sample JSON Output | ||
|
|
||
| Given the following Test Case | ||
|
|
||
| ```swift | ||
| extention Tag { | ||
| public static var blue: Self { | ||
| Tag(kind: .staticMember("blue")) | ||
| } | ||
|
|
||
| /// A tag representing the color red. | ||
| public static var red: Self { | ||
| Tag(kind: .staticMember("red")) | ||
| } | ||
| } | ||
|
|
||
| @Test( | ||
| .tags(.blue), | ||
| .tags(Tag.red), | ||
| .bug("https://my.defect.com/1234"), | ||
| .bug("other defect"), | ||
| .timeLimit(Swift.Duration.seconds(testTimeLimit + 100)), | ||
| .timeLimit(Swift.Duration.seconds(testTimeLimit)), | ||
| .timeLimit(Swift.Duration.seconds(testTimeLimit + 10)), | ||
| arguments: expectedArgs as [String] | ||
| ) | ||
| func example {} | ||
| ``` | ||
|
|
||
| The proposed JSON containing the new fields would looks like | ||
|
|
||
| ```json | ||
| { | ||
| "kind": "test", | ||
| "payload": { | ||
| <...SNIP...>, | ||
| "bugs": [ | ||
| { | ||
| "url": "https:\/\/my.defect.com\/1234" | ||
| }, | ||
| { | ||
| "url": "other defect" | ||
| } | ||
| ], | ||
| "tags": [ | ||
| ".blue", | ||
| ".red" | ||
| ], | ||
| "timeLimit": 3 | ||
| }, | ||
| } | ||
| ``` | ||
|
|
||
| ## Source compatibility | ||
|
|
||
| This proposal maintains full backward compatibility through careful design: | ||
|
|
||
| - **ABI Version Protection**: New fields are conditionally included based on ABI | ||
| version checks, ensuring older tools continue to function without modification | ||
| - **Experimental Feature Migration**: The existing experimental `_tags` field is | ||
| replaced with the `tags` array. Since experimental features don't provide | ||
| stability guarantees, this replacement doesn't constitute a breaking change | ||
| - **Graceful Degradation**: Tools that don't expect the new fields will simply ignore | ||
| them, while updated tools can leverage the enhanced metadata | ||
|
|
||
| No existing functionality is affected, making this a purely additive enhancement. | ||
|
|
||
| ## Integration with supporting tools | ||
|
|
||
| The enhanced JSON ABI opens up exciting possibilities for the Swift Testing ecosystem: | ||
|
|
||
| ### Immediate Benefits for Tool Developers | ||
| - **IDE Extensions**: Can now provide tag-based test filtering and organization | ||
| - **CI/CD Integrations**: Can generate more detailed reports with test categorization | ||
| - **Performance Monitoring**: Can track and alert on time limit violations | ||
| - **Bug Tracking Integration**: Can correlate test results with known issues | ||
|
|
||
| ### Migration Path | ||
| Existing tools will continue to work unchanged, as the new fields are purely additive. | ||
| Tool developers can incrementally adopt the enhanced metadata at their own pace, | ||
| choosing which fields provide the most value for their specific use cases. | ||
|
|
||
| ## Future directions | ||
|
|
||
| This enhancement establishes future richer tooling experiences: | ||
|
|
||
| ### Alternative Field Naming | ||
| - **`timeLimitInSeconds` vs `timeLimit`**: We chose the shorter `timeLimit` name for | ||
| consistency with Swift Testing's existing API, with the time unit documented in the | ||
| schema specification. The naming convention was discussed with the Testing Workgroup | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this blurb about renaming the time fields move to Future directions?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved |
||
| and it was decided that a seperata proposal should be made on how to represent | ||
| the time units in the name/value. | ||
|
|
||
| ### Potential Extensions | ||
| - **Additional Metadata**: Other test traits could be exposed as the ecosystem evolves | ||
|
|
||
| ## Alternatives considered | ||
|
|
||
| ### Alternative Data Structures | ||
| - **Flattened vs Structured Bug Information**: We chose a structured approach for bug | ||
| metadata to accommodate various bug tracking systems while maintaining extensibility | ||
|
|
||
| ### Unconditionally include optional field | ||
| - We selected conditional inclusion to keep JSON output clean and avoid null values, | ||
| improving the developer experience for tools consuming the data. | ||
|
|
||
| ## Acknowledgments | ||
|
|
||
| Thanks to [Jonathan Grynspan](https://github.com/grynspan) for suggesting to me | ||
| I write this proposal and for providing feedback. | ||
|
|
||
| Thanks to [Paul LeMarqaund](https://github.com/plemarquand) for providing proposal | ||
| feedback before it was posted. | ||
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should think about whether the leading period should be encoded in the JSON. Since this schema may be used by other libraries that don't use Swift's dot syntax, my gut says they shouldn't be encoded and we should just define
<tag> ::= <string>.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a strong opinion here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree w/ @grynspan and think we shouldn't necessarily specify in the schema itself that there will always be a leading period character. But I do think the proposal should clearly specify whether, in the encoded string, the leading period and any subsequent punctuation will be preserved as it was written in source. The main thing I'm trying to convey is that we ought to clearly define the behavior, whatever it is.
And on this specific point, I would suggest you define the behavior such that it will precisely preserve the way the tag was spelled in source. So if the code had
.tags(.blue)the string would be".blue". If it had.tags(Tag.red), the string would be"Tag.red", and so on. The latter example differs from the example you show in the current proposal, so that represents a change.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be the wrong abstraction though, as it would mean
Tag.redand.reddon't compare equal. (I don't think that string is available at runtime anyway!) Furthermore, preserving the source would prevent the use of this feature with libraries that define tags using different syntax. For example, C++ might have something using::, or Objective-C might just use@"red".I'd like to just define it formally as
<string>and document that Swift Testing specifically normalizes to"red"since that would be most compatible with what other libraries or tagging syntaxes would reasonably do.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm open to that. To be comprehensive about this, how would examples which include interstitial punctuation, like
.tags(.NestedType.bar, .NestedType.AnotherNestedType.baz)be encoded in the JSON? As"NestedType.bar"and"NestedType.AnotherNestedType.baz", respectively, or something else?In effect, this will mean we'll be treating the optional
(Testing.)Tagtype namespace prefixes as "special", and stripping them when present, along with the leading period. Which is fine, but it should be spelled out in the proposal. (Maybe it would be helpful to include a Markdown table with several of these examples.)Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already strip "Testing.Tag." when ingesting the tag at compile time, and we don't show it in the Xcode UI. Specifically for Swift Testing, I'd say the tag
.foo.barshould be rendered as it is in Xcode, i.e. as "foo.bar". I recognize a hypothetical C++ library might have trouble with that spelling if they're trying to be source-accurate.On the other hand, AFAIK one-level tag names (
.red) are overwhelmingly more common in practice, so that could just be a corner case.