Skip to content

Improve diagnostic output for trace2 data assertions#2159

Open
spkrka wants to merge 2 commits into
gitgitgadget:masterfrom
spkrka:test-diagnostics
Open

Improve diagnostic output for trace2 data assertions#2159
spkrka wants to merge 2 commits into
gitgitgadget:masterfrom
spkrka:test-diagnostics

Conversation

@spkrka

@spkrka spkrka commented Jun 26, 2026

Copy link
Copy Markdown

While working on trace2 instrumentation for merge-base computation,
I found it tricky to fix up expected values when assertions failed --
test_trace2_data is a bare grep that silently returns a non-zero
exit code, giving no indication of what went wrong.

This series adds test_trace2_data_singular, a more informative
variant that verifies the event appears exactly once and reports
clear diagnostics on failure (key not found, multiple entries, or
value mismatch), then migrates all applicable call sites to use it.

The migration also exposed a latent trace file collision in t5620
where two tests wrote to the same file and GIT_TRACE2_EVENT
silently appended, which is fixed in the second patch.

Before (value mismatch, with -v):

$ test_trace2_data status count/changed 999 <trace2.txt
$ echo $?
1
(no output)

After:

$ test_trace2_data_singular status count/changed 999 <trace2.txt
error: trace2 data 'status/count/changed'
  expected: 999
  actual:   0

cc: Patrick Steinhardt ps@pks.im

spkrka added 2 commits June 26, 2026 14:26
test_trace2_data is a bare grep that silently exits on failure.
Add a more informative variant that verifies the event appears
exactly once and reports what went wrong: key not found, multiple
entries, or value mismatch.  Diagnostics go to FD 4 like test_grep.

Before (value mismatch):

  $ test_trace2_data status count/changed 999 <trace2.txt
  $ echo $?
  1
  (no output)

After:

  $ test_trace2_data_singular status count/changed 999 <trace2.txt
  error: trace2 data 'status/count/changed'
    expected: 999
    actual:   0

Signed-off-by: Kristofer Karlsson <krka@spotify.com>
Switch all call sites that expect a single occurrence to use the
new function.  The two calls in t5620 test 7 ("backfill min batch
size") stay on the old function since they expect multiple entries.

Rename the trace file in t5620 test 14 to avoid a collision with
test 13 that the stricter check exposed.

Signed-off-by: Kristofer Karlsson <krka@spotify.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant