Page MenuHomePhabricator

test: stabilize test-run-tests.t output
ClosedPublic

Authored by lothiraldan on Feb 14 2019, 9:40 AM.

Details

Summary

We have reached a point where the duration in JSON reports of
test-run-tests.t were greater or equal than 10 seconds, which doesn't match
anymore the regex. For example here:
https://ci.octobus.net/blue/organizations/jenkins/MercurialPy2/detail/MercurialPy2/276/pipeline

           "diff": "", ? (re)
-          "end": "\s*[\d\.]{4,5}", ? (re)
+          "end": "10.040",
           "result": "skip", ? (re)

Instead of accepting more characters, I changed the regex to accept any number
of digits before the . then 3 or 4 digits after. This way the regex is more
precise (only one . is authorized and we can ensure that the precision
doesn't change).

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

lothiraldan created this revision.Feb 14 2019, 9:40 AM
durin42 added inline comments.
tests/test-run-tests.t
1177–1178

Maybe we could make it 3,5 so it's be more permissive?

lothiraldan added inline comments.Feb 18 2019, 4:29 AM
tests/test-run-tests.t
1177–1178

Yes we could.

I think the regex was previously {4,5} for two reasons:

  • We expected the duration to be less than 10 seconds.
  • The precision of the number was 3 digits or 4 digits after the dot depending if we used json or simplejson for dumping the data.

Right now, it seems that run-tests.py itself is limitation the precision of the digits.

So we can either drop the {3,4} as we should have a stable number of digits after the dot, and anyone changing the precision of times in run-tests.py will have a diff in this test.

Or we make it more permissive so we don't need to bother about updating it later.

I don't have a strong opinion about either, what do you think we should do?

durin42 added inline comments.Feb 19 2019, 10:10 AM
tests/test-run-tests.t
1177–1178

I don't feel strongly, let's just make it permissive so the test passes on both fast and slow systems.

lothiraldan added inline comments.Feb 19 2019, 1:29 PM
tests/test-run-tests.t
1177–1178

I think it's the case, the first part of the regex \d+ will match any amount of seconds and the remaining part \.\d{3,4} should match as long the precision doesn't change.

durin42 added inline comments.Feb 20 2019, 3:09 PM
tests/test-run-tests.t
1177–1178

Ah I see, I wasn't reading the change in regular expression quite right the first few times. Ugh.

This revision was automatically updated to reflect the committed changes.