Buildkite Test Analytics: fix failure_expanded

[pda]

This pull request changes failure_expanded from Dict{String, Any} to Vector{Dict{String, Any}} to fix a JSON schema issue in the Bootleg JSON writer 🏴‍☠️ which is responsible for producing JSON test results for Buildkite Test Analytics.

The failure_expanded attribute of a test result is a JSON array of objects, rather than a single object. I believe this is to support the possibility of multiple failure reasons in a single test case, although I'm not sure if that's used anywhere.

I believe the associated uploads (batches of up to 5,000 results) are currently getting a successful HTTP 202 Accepted response, but the response body will contain an error for each test case that had a non-array failure_expanded, meaning those test cases will be dropped:

HTTP/1.1 202 Accepted
Content-Type: application/json; charset=utf-8
Date: Tue, 12 Mar 2024 13:11:46 GMT
X-Request-Id: a35322f6-9990-4b8e-8895-d62bd6e10935

{
  "id": "55ac3b92-…",
  "run_id": "bfa6de98-…",
  "queued": 4998,
  "skipped": 2,
  "errors": [
    "Validation failed: Failure expanded must be an Array",
    "Validation failed: Failure expanded must be an Array"
  ],
  "run_url": "http://buildkite.com/…"
}

Rather than make the Buildkite API more permissive, I figured I'd come and fix it upstream, and write my first few tiny lines of Julia while I'm at it 😁

I've verified that the adjusted JSON output it accepted by our ingestion system.

---

For verification, I added an error to an arbitrarily selected test (because workers don't return information about passing/broken tests, only errors or failure):

diff --git a/test/char.jl b/test/char.jl
index 1d3579013a..582423e8a3 100644
--- a/test/char.jl
+++ b/test/char.jl
@@ -1,6 +1,7 @@
 # This file is a part of Julia. License is MIT: https://julialang.org/license

 @testset "basic properties" begin
+    @test throw(ErrorException("testing Buildkite JSON"))
     @test typemax(Char) == reinterpret(Char, typemax(UInt32))
     @test typemin(Char) == Char(0)
     @test typemax(Char) == reinterpret(Char, 0xffffffff)

… and then CI=true ./julia test/runtests.jl char which produces test/results_1.json.

Before:

[
  {
    "file_name": "/Users/pda/code/julia/test/char.jl",
    "history": {
      "duration": 2.123565912246704,
      "start_at": 1.710245465232599e9,
      "end_at": 1.710245467356165e9
    },
    "name": "test_error: throw(ErrorException(\"testing Buildkite JSON\"))",
    "location": "/Users/pda/code/julia/test/char.jl:4",
    "failure_reason": "Exception (unexpectedly) thrown during test",
    "scope": "/Overall/char",
    "failure_expanded": {
      "backtrace": [
        " [1] top-level scope",
        "   @ ~/code/julia/test/char.jl:4",
        " [2] macro expansion",
        "   @ ~/code/julia/usr/share/julia/stdlib/v1.12/Test/src/Test.jl:164 [inlined]",
        " [3] macro expansion",
        "   @ ~/code/julia/test/char.jl:4 [inlined]",
        " [4] macro expansion",
        "   @ ~/code/julia/usr/share/julia/stdlib/v1.12/Test/src/Test.jl:164 [inlined]",
        " [5] macro expansion",
        "   @ ~/code/julia/test/char.jl:4 [inlined]"
      ],
      "expanded": [
        "testing Buildkite JSON"
      ]
    },
    "id": "e9272117-d5f5-f542-039b-cfd3d2e8f33a",
    "result": "failed"
  }
]

After:

[
  {
    "file_name": "/Users/pda/code/julia/test/char.jl",
    "history": {
      "duration": 2.123565912246704,
      "start_at": 1.710245465232599e9,
      "end_at": 1.710245467356165e9
    },
    "name": "test_error: throw(ErrorException(\"testing Buildkite JSON\"))",
    "location": "/Users/pda/code/julia/test/char.jl:4",
    "failure_reason": "Exception (unexpectedly) thrown during test",
    "scope": "/Overall/char",
    "failure_expanded": [
      {
        "backtrace": [
          " [1] top-level scope",
          "   @ ~/code/julia/test/char.jl:4",
          " [2] macro expansion",
          "   @ ~/code/julia/usr/share/julia/stdlib/v1.12/Test/src/Test.jl:164 [inlined]",
          " [3] macro expansion",
          "   @ ~/code/julia/test/char.jl:4 [inlined]",
          " [4] macro expansion",
          "   @ ~/code/julia/usr/share/julia/stdlib/v1.12/Test/src/Test.jl:164 [inlined]",
          " [5] macro expansion",
          "   @ ~/code/julia/test/char.jl:4 [inlined]"
        ],
        "expanded": [
          "testing Buildkite JSON"
        ]
      }
    ],
    "id": "e9272117-d5f5-f542-039b-cfd3d2e8f33a",
    "result": "failed"
  }
]

Diff:

--- buildkite-before.json	2024-03-12 23:08:26
+++ buildkite-after.json	2024-03-12 23:07:58
@@ -10,23 +10,25 @@
     "location": "/Users/pda/code/julia/test/char.jl:4",
     "failure_reason": "Exception (unexpectedly) thrown during test",
     "scope": "/Overall/char",
-    "failure_expanded": {
-      "backtrace": [
-        " [1] top-level scope",
-        "   @ ~/code/julia/test/char.jl:4",
-        " [2] macro expansion",
-        "   @ ~/code/julia/usr/share/julia/stdlib/v1.12/Test/src/Test.jl:164 [inlined]",
-        " [3] macro expansion",
-        "   @ ~/code/julia/test/char.jl:4 [inlined]",
-        " [4] macro expansion",
-        "   @ ~/code/julia/usr/share/julia/stdlib/v1.12/Test/src/Test.jl:164 [inlined]",
-        " [5] macro expansion",
-        "   @ ~/code/julia/test/char.jl:4 [inlined]"
-      ],
-      "expanded": [
-        "testing Buildkite JSON"
-      ]
-    },
+    "failure_expanded": [
+      {
+        "backtrace": [
+          " [1] top-level scope",
+          "   @ ~/code/julia/test/char.jl:4",
+          " [2] macro expansion",
+          "   @ ~/code/julia/usr/share/julia/stdlib/v1.12/Test/src/Test.jl:164 [inlined]",
+          " [3] macro expansion",
+          "   @ ~/code/julia/test/char.jl:4 [inlined]",
+          " [4] macro expansion",
+          "   @ ~/code/julia/usr/share/julia/stdlib/v1.12/Test/src/Test.jl:164 [inlined]",
+          " [5] macro expansion",
+          "   @ ~/code/julia/test/char.jl:4 [inlined]"
+        ],
+        "expanded": [
+          "testing Buildkite JSON"
+        ]
+      }
+    ],
     "id": "e9272117-d5f5-f542-039b-cfd3d2e8f33a",
     "result": "failed"
   }

[pda]

Pinging @tecosaur as the main author of test/buildkitetestjson.jl

[tecosaur]

Hello @pda, thanks for raising this issue and coming in with a fix too (with such a detailed explanation too 😍)!

Looking at the code, it all looks good to me 🙂

Personally, I'd prefer it if the Buildkit -> BuildKite change + typo fix was a separate git commit, but PRs here seem to tend to be squash-merged, so it's probably not worth bothering separating out these different changes here.

As a (personal) style nit, I'm not sure I'm a fan of the closing braces on their own lines, and would be tempted to do a more compact style like this

data["failure_expanded"] =
    [Dict{String,Any}("expanded" => split(err, '\n'),
                      "backtrace" => split(trace, '\n'))]

but I think at this point we're more in the realm of personal preference than style guides.

So, this looks good to me and thanks again for the contribution!

[pda]

Personally, I'd prefer it if the Buildkit -> BuildKite change + typo fix was a separate git commit

Done!

As a (personal) style nit, I'm not sure I'm a fan of the closing braces on their own lines, and would be tempted to do a more compact style like this

I'll absolutely defer to you on Julia style, it's not my home turf at all.
Done!

[pda]

Aside: I notice the environment var CI is interpreted as boolean, and so the JSON files are produced with CI=true but not CI=buildkite, and the latter is what I would expect in actual CI.

julia/test/runtests.jl

Line 431 in e99627f

if Base.get_bool_env("CI", false)

But it seems to be working, so perhaps the environment is being modified somewhere within the test running scripts etc?
Probably not a problem! Just an observation.

This is what I ran to generate the files locally:

CI=true ./julia test/runtests.jl char

[vchuravy]

Thank you @pda!