Page MenuHomePhabricator

test: stabilize test-run-tests.t output

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



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:

           "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

rHG Mercurial
Automatic diff as part of commit; lint not applicable.
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.

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

lothiraldan added inline comments.Feb 18 2019, 4:29 AM

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

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

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

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.