bryancall opened a new issue, #13664:
URL: https://github.com/apache/trafficserver/issues/13664

   ## 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.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to