[Pitch] Adding additional information to the ABI JSON

Evolution Pitches
1

Hi folks,

I hope you are all well. What do folks think about augmenting the event JSON ABI to include additional information test information, such as bug. timeLimit and tags (which is included in the experimental ABI JSON).

Motivation

The event JSON stream offers great data about the Swift Testing tests. Tools can be written to extract information from the JSON. However, the event JSON stream is currently missing some traits information about the test itself. This information is can be important for tools wanting to use the missing information.

Proposed solution

We propose adding tags, bug and timeLimit to the test payload where the Modified Backus-Naur Form (BNF) delta would be:

diff --git a/Documentation/ABI/JSON.md b/Documentation/ABI/JSON.md
index e4ff24a..1a82996 100644
--- a/Documentation/ABI/JSON.md
+++ b/Documentation/ABI/JSON.md
@@ -157,10 +157,26 @@ 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
+
+<tags> ::= <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
+} ;
+

An example output of the test kind is

{
    "kind":"test",
    "payload":{
        "tags": [".purple",".blue",".red",".green",".orange",".traitRelated",".yellow"],
        "bugs":[
            {
                "url": "myInternalScheme://defect/9876"
            },
            {
                "url": "http://example.com/defect/1234",
                "title": "another defect"
            }
        ],
        "timeLimit": 5,
        <...SNIP...>
    },
    <...SNIP...>
}

I have a draft PR which add this functionality, which guards the new fields behind an ABI version.

Looking forward to your feedback.

Sam

EDIT: I made some changes to the proposed Modified BNF changes

1 Like
2

The timeLimit is currently in seconds. Should the property be renamed to include the units in the name to make it less ambiguous? e.g.: timeLimitInSeconds instead of timeLimit?

3

For anyone interested, the proposal PR has been published. Here is a human-readable version.

4

I’m a big fan of exposing more of the traits that provide rich context for tests for tools to use.

I just wish Xcode would expose more of this information…

RE: timeLimitInSeconds I’m in favor, just to avoid ambiguity for posterity

5

Side note about timeLimit — just to clarify, the API only supports expressing time limits in terms of minutes, but the ABI JSON will resurface that in terms of seconds?

6

All other time values in this schema are in seconds, so IMHO it's redundant information to explicitly encode it in the key here. (Conciseness is a virtue in the schema itself.)

The minute limit is specific to Swift Testing, whereas this JSON schema can (and will eventually be) used by other libraries. And, as I mentioned, we already report time values in seconds elsewhere.

7

Hi. I presented the pitch to the Testing Workgroup. Although everyone was in favour is being explicit with the units, the omission of the units in the name was the chosen outcome for this pitch for the sake of consistency and to reduce the amount of work as the ABI JSON does not mention units in the property names.

We also discussed that a separate pitch/proposal should be done to address the time units across the entire ABI JSON in a consistent manner. I updated the proposal with this information.

8

For what it's worth, if/when this pitch proceeds to the review stage, the Testing Workgroup will take feedback from the public: if there's a strong desire for a different name, that's not going to be ignored.

3 Likes
9

Ah, I wasn’t aware of that context.

Then I’m fine with it as-is.

10

Thank you for the pitch @bkhouri!

Consider me fully in favor: it will be clearly useful to include these details in the event stream JSON representation.

To help push this forward, I have given the proposal document a thorough review and left a variety of comments on the proposal PR. They range from minor (formatting and editing) to more substantial things like the precise representation of tags and the title of the proposal itself. Please have a look when you have time!

Stuart

2 Likes
11

Thank you very much for you feedback @smontgomery . I believe I addressed most, if not all, of your comments. Though I do not explicitly mention the tag "structure" in the proposal, I did augment the BNF change to indicate the tags start with a . and also provided a sample JSON.

See SwiftTesting: Include metadata for tags, bug, and time limit traits in event stream by bkhouri · Pull Request #3040 · swiftlang/swift-evolution · GitHub.

Could I ask that you please review my replies in the proposal PR, and resolve any comments that you deem are completed?

Thanks again for the feedback.

Sam

12

With updating the title, the human readable proposal location moved to swift-evolution/proposals/testing/NNNN-include-tags-bugs-and-timeline-in-event-stream.md at t/main/augment_event_stream_json · bkhouri/swift-evolution · GitHub

13

I added some technical-nitty-gritty feedback to the SE PR. Apologies if any of it is redundant vs. the implementation PR (we'll make sure everything is harmonized before it gets merged).

1 Like