Summary
connect_attempts fails intermittently. The gold file pins the HTTP state machine id
of each transaction, which is an ordering artifact rather than part of the behaviour
the test is asserting.
Observed
CI build 43249, AuTest 0of4, 1 failed out of 146 tests. Every other assertion in the
run passed; the failure is a gold diff:
- ... for host='example.com' sm_id=0 ... retry_attempts=0 ... (gold)
+ ... for host='example.com' sm_id=2 ... retry_attempts=0 ... (actual)
- ... url='...' fail_count='3' marking down (gold)
+ ... url='...' fail_count='2' marking down (actual)
Retriggering the identical commit passed, so it is not deterministic.
Cause
tests/gold_tests/dns/gold/connect_attempts_single_max_retries_error_log.gold
wildcards the timestamp and the port on every line, but spells the state machine id
out literally:
`` CONNECT: attempt fail [CONNECTION_ERROR] to 0.0.0.1:`` for host='example.com' sm_id=0 `` retry_attempts=0 url='...'
`` CONNECT: attempt fail [CONNECTION_ERROR] to 0.0.0.1:`` for host='example.com' sm_id=2 `` retry_attempts=0 url='...'
The test drives two transactions. The gold assumes the one that gets sm_id=0 always
logs its retries before the one that gets sm_id=2. When the two interleave the other
way the comparison fails, even though the retry behaviour under test was correct.
fail_count has the same character: it is a counter sampled at a moment, not the
property the test exists to check.
Testers.GoldFile compares line by line and has no ordering option, so the gold itself
is where this has to be addressed.
Suggested fix
Wildcard the state machine id the same way the timestamp and port already are, so the
gold asserts what the test is about (two transactions, each retried the configured
number of times) rather than which id they were assigned. If the count of distinct
transactions matters, that is better expressed as a separate assertion than by pinning
the ids.
If preserving the ids is deliberate, Testers.GoldFileList accepts several golds and
passes on any of them, which would cover both interleavings.
Notes
Found while testing an unrelated change, #13661, whose diff is limited to
src/tsutil/Regex.cc, its unit test, and the regex_remap autest. connect_attempts
does not use regex, and the test passed five out of five locally on that branch.
Same class as #13662: a test asserting an incidental runtime value.
Summary
connect_attemptsfails intermittently. The gold file pins the HTTP state machine idof each transaction, which is an ordering artifact rather than part of the behaviour
the test is asserting.
Observed
CI build 43249, AuTest 0of4, 1 failed out of 146 tests. Every other assertion in the
run passed; the failure is a gold diff:
Retriggering the identical commit passed, so it is not deterministic.
Cause
tests/gold_tests/dns/gold/connect_attempts_single_max_retries_error_log.goldwildcards the timestamp and the port on every line, but spells the state machine id
out literally:
The test drives two transactions. The gold assumes the one that gets
sm_id=0alwayslogs its retries before the one that gets
sm_id=2. When the two interleave the otherway the comparison fails, even though the retry behaviour under test was correct.
fail_counthas the same character: it is a counter sampled at a moment, not theproperty the test exists to check.
Testers.GoldFilecompares line by line and has no ordering option, so the gold itselfis where this has to be addressed.
Suggested fix
Wildcard the state machine id the same way the timestamp and port already are, so the
gold asserts what the test is about (two transactions, each retried the configured
number of times) rather than which id they were assigned. If the count of distinct
transactions matters, that is better expressed as a separate assertion than by pinning
the ids.
If preserving the ids is deliberate,
Testers.GoldFileListaccepts several golds andpasses on any of them, which would cover both interleavings.
Notes
Found while testing an unrelated change, #13661, whose diff is limited to
src/tsutil/Regex.cc, its unit test, and theregex_remapautest.connect_attemptsdoes not use regex, and the test passed five out of five locally on that branch.
Same class as #13662: a test asserting an incidental runtime value.