This is an archive of the discontinued Mercurial Phabricator instance.

Differential D5966

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

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

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG0ae89ab3f025: test: stabilize test-run-tests.t output
rHGd8ac04c3c74d: test: stabilize test-run-tests.t output
rHG4618bdf75d8a: test: stabilize test-run-tests.t output
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 a subscriber: durin42.Feb 15 2019, 10:58 PM
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.

Closed by commit rHG4618bdf75d8a: test: stabilize test-run-tests.t output (authored by lothiraldan). · Explain WhyFeb 20 2019, 3:16 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mtests/test-run-tests.t (90 lines)

Diff 14146

tests/test-run-tests.t

Failed test-failure.t: output changed Failed test-failure.t: output changed
# Ran 2 tests, 1 skipped, 1 failed. # Ran 2 tests, 1 skipped, 1 failed.
python hash seed: * (glob) python hash seed: * (glob)
[1] [1]
$ cat report.json $ cat report.json
testreport ={ testreport ={
"test-failure.t": [\{] (re) "test-failure.t": [\{] (re)
"csys": "\s*[\d\.]{4,5}", ? (re) "csys": "\s*\d+\.\d{3,4}", ? (re)
"cuser": "\s*[\d\.]{4,5}", ? (re) "cuser": "\s*\d+\.\d{3,4}", ? (re)
durin42Unsubmitted
Not Done

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

durin42: Maybe we could make it 3,5 so it's be more permissive?
lothiraldanAuthorUnsubmitted
Not Done

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?

lothiraldan: Yes we could. I think the regex was previously `{4,5}` for two reasons: - We expected the…
durin42Unsubmitted
Not Done

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

durin42: I don't feel strongly, let's just make it permissive so the test passes on both fast and slow…
lothiraldanAuthorUnsubmitted
Not Done

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.

lothiraldan: I think it's the case, the first part of the regex `\d+` will match any amount of seconds and…
durin42Unsubmitted
Not Done

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

durin42: Ah I see, I wasn't reading the change in regular expression quite right the first few times.
"diff": "---.+\+\+\+.+", ? (re) "diff": "---.+\+\+\+.+", ? (re)
"end": "\s*[\d\.]{4,5}", ? (re) "end": "\s*\d+\.\d{3,4}", ? (re)
"result": "failure", ? (re) "result": "failure", ? (re)
"start": "\s*[\d\.]{4,5}", ? (re) "start": "\s*\d+\.\d{3,4}", ? (re)
"time": "\s*[\d\.]{4,5}" (re) "time": "\s*\d+\.\d{3,4}" (re)
}, ? (re) }, ? (re)
"test-skip.t": { "test-skip.t": {
"csys": "\s*[\d\.]{4,5}", ? (re) "csys": "\s*\d+\.\d{3,4}", ? (re)
"cuser": "\s*[\d\.]{4,5}", ? (re) "cuser": "\s*\d+\.\d{3,4}", ? (re)
"diff": "", ? (re) "diff": "", ? (re)
"end": "\s*[\d\.]{4,5}", ? (re) "end": "\s*\d+\.\d{3,4}", ? (re)
"result": "skip", ? (re) "result": "skip", ? (re)
"start": "\s*[\d\.]{4,5}", ? (re) "start": "\s*\d+\.\d{3,4}", ? (re)
"time": "\s*[\d\.]{4,5}" (re) "time": "\s*\d+\.\d{3,4}" (re)
}, ? (re) }, ? (re)
"test-success.t": [\{] (re) "test-success.t": [\{] (re)
"csys": "\s*[\d\.]{4,5}", ? (re) "csys": "\s*\d+\.\d{3,4}", ? (re)
"cuser": "\s*[\d\.]{4,5}", ? (re) "cuser": "\s*\d+\.\d{3,4}", ? (re)
"diff": "", ? (re) "diff": "", ? (re)
"end": "\s*[\d\.]{4,5}", ? (re) "end": "\s*\d+\.\d{3,4}", ? (re)
"result": "success", ? (re) "result": "success", ? (re)
"start": "\s*[\d\.]{4,5}", ? (re) "start": "\s*\d+\.\d{3,4}", ? (re)
"time": "\s*[\d\.]{4,5}" (re) "time": "\s*\d+\.\d{3,4}" (re)
} }
} (no-eol) } (no-eol)
--json with --outputdir --json with --outputdir
$ rm report.json $ rm report.json
$ rm -r output $ rm -r output
$ mkdir output $ mkdir output
$ rt --json --outputdir output $ rt --json --outputdir output
# Ran 2 tests, 1 skipped, 1 failed. # Ran 2 tests, 1 skipped, 1 failed.
python hash seed: * (glob) python hash seed: * (glob)
[1] [1]
$ f report.json $ f report.json
report.json: file not found report.json: file not found
$ cat output/report.json $ cat output/report.json
testreport ={ testreport ={
"test-failure.t": [\{] (re) "test-failure.t": [\{] (re)
"csys": "\s*[\d\.]{4,5}", ? (re) "csys": "\s*\d+\.\d{3,4}", ? (re)
"cuser": "\s*[\d\.]{4,5}", ? (re) "cuser": "\s*\d+\.\d{3,4}", ? (re)
"diff": "---.+\+\+\+.+", ? (re) "diff": "---.+\+\+\+.+", ? (re)
"end": "\s*[\d\.]{4,5}", ? (re) "end": "\s*\d+\.\d{3,4}", ? (re)
"result": "failure", ? (re) "result": "failure", ? (re)
"start": "\s*[\d\.]{4,5}", ? (re) "start": "\s*\d+\.\d{3,4}", ? (re)
"time": "\s*[\d\.]{4,5}" (re) "time": "\s*\d+\.\d{3,4}" (re)
}, ? (re) }, ? (re)
"test-skip.t": { "test-skip.t": {
"csys": "\s*[\d\.]{4,5}", ? (re) "csys": "\s*\d+\.\d{3,4}", ? (re)
"cuser": "\s*[\d\.]{4,5}", ? (re) "cuser": "\s*\d+\.\d{3,4}", ? (re)
"diff": "", ? (re) "diff": "", ? (re)
"end": "\s*[\d\.]{4,5}", ? (re) "end": "\s*\d+\.\d{3,4}", ? (re)
"result": "skip", ? (re) "result": "skip", ? (re)
"start": "\s*[\d\.]{4,5}", ? (re) "start": "\s*\d+\.\d{3,4}", ? (re)
"time": "\s*[\d\.]{4,5}" (re) "time": "\s*\d+\.\d{3,4}" (re)
}, ? (re) }, ? (re)
"test-success.t": [\{] (re) "test-success.t": [\{] (re)
"csys": "\s*[\d\.]{4,5}", ? (re) "csys": "\s*\d+\.\d{3,4}", ? (re)
"cuser": "\s*[\d\.]{4,5}", ? (re) "cuser": "\s*\d+\.\d{3,4}", ? (re)
"diff": "", ? (re) "diff": "", ? (re)
"end": "\s*[\d\.]{4,5}", ? (re) "end": "\s*\d+\.\d{3,4}", ? (re)
"result": "success", ? (re) "result": "success", ? (re)
"start": "\s*[\d\.]{4,5}", ? (re) "start": "\s*\d+\.\d{3,4}", ? (re)
"time": "\s*[\d\.]{4,5}" (re) "time": "\s*\d+\.\d{3,4}" (re)
} }
} (no-eol) } (no-eol)
$ ls -a output $ ls -a output
. .
.. ..
.testtimes .testtimes
report.json report.json
test-failure.t.err test-failure.t.err
pad pad pad pad............................................................ pad pad pad pad............................................................
Accept this change? [n] ..s Accept this change? [n] ..s
Skipped test-skip.t: missing feature: nail clipper Skipped test-skip.t: missing feature: nail clipper
# Ran 2 tests, 1 skipped, 0 failed. # Ran 2 tests, 1 skipped, 0 failed.
$ cat report.json $ cat report.json
testreport ={ testreport ={
"test-failure.t": [\{] (re) "test-failure.t": [\{] (re)
"csys": "\s*[\d\.]{4,5}", ? (re) "csys": "\s*\d+\.\d{3,4}", ? (re)
"cuser": "\s*[\d\.]{4,5}", ? (re) "cuser": "\s*\d+\.\d{3,4}", ? (re)
"diff": "", ? (re) "diff": "", ? (re)
"end": "\s*[\d\.]{4,5}", ? (re) "end": "\s*\d+\.\d{3,4}", ? (re)
"result": "success", ? (re) "result": "success", ? (re)
"start": "\s*[\d\.]{4,5}", ? (re) "start": "\s*\d+\.\d{3,4}", ? (re)
"time": "\s*[\d\.]{4,5}" (re) "time": "\s*\d+\.\d{3,4}" (re)
}, ? (re) }, ? (re)
"test-skip.t": { "test-skip.t": {
"csys": "\s*[\d\.]{4,5}", ? (re) "csys": "\s*\d+\.\d{3,4}", ? (re)
"cuser": "\s*[\d\.]{4,5}", ? (re) "cuser": "\s*\d+\.\d{3,4}", ? (re)
"diff": "", ? (re) "diff": "", ? (re)
"end": "\s*[\d\.]{4,5}", ? (re) "end": "\s*\d+\.\d{3,4}", ? (re)
"result": "skip", ? (re) "result": "skip", ? (re)
"start": "\s*[\d\.]{4,5}", ? (re) "start": "\s*\d+\.\d{3,4}", ? (re)
"time": "\s*[\d\.]{4,5}" (re) "time": "\s*\d+\.\d{3,4}" (re)
}, ? (re) }, ? (re)
"test-success.t": [\{] (re) "test-success.t": [\{] (re)
"csys": "\s*[\d\.]{4,5}", ? (re) "csys": "\s*\d+\.\d{3,4}", ? (re)
"cuser": "\s*[\d\.]{4,5}", ? (re) "cuser": "\s*\d+\.\d{3,4}", ? (re)
"diff": "", ? (re) "diff": "", ? (re)
"end": "\s*[\d\.]{4,5}", ? (re) "end": "\s*\d+\.\d{3,4}", ? (re)
"result": "success", ? (re) "result": "success", ? (re)
"start": "\s*[\d\.]{4,5}", ? (re) "start": "\s*\d+\.\d{3,4}", ? (re)
"time": "\s*[\d\.]{4,5}" (re) "time": "\s*\d+\.\d{3,4}" (re)
} }
} (no-eol) } (no-eol)
$ mv backup test-failure.t $ mv backup test-failure.t
backslash on end of line with glob matching is handled properly backslash on end of line with glob matching is handled properly
$ cat > test-glob-backslash.t << EOF $ cat > test-glob-backslash.t << EOF
> $ echo 'foo bar \\' > $ echo 'foo bar \\'