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

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

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

jeffpc (Jeff Sipek)
lothiraldan created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION 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).

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5966

AFFECTED FILES
  tests/test-run-tests.t

CHANGE DETAILS

diff --git a/tests/test-run-tests.t b/tests/test-run-tests.t
--- a/tests/test-run-tests.t
+++ b/tests/test-run-tests.t
@@ -1174,31 +1174,31 @@
   $ cat report.json
   testreport ={
       "test-failure.t": [\{] (re)
-          "csys": "\s*[\d\.]{4,5}", ? (re)
-          "cuser": "\s*[\d\.]{4,5}", ? (re)
+          "csys": "\s*\d+\.\d{3,4}", ? (re)
+          "cuser": "\s*\d+\.\d{3,4}", ? (re)
           "diff": "---.+\+\+\+.+", ? (re)
-          "end": "\s*[\d\.]{4,5}", ? (re)
+          "end": "\s*\d+\.\d{3,4}", ? (re)
           "result": "failure", ? (re)
-          "start": "\s*[\d\.]{4,5}", ? (re)
-          "time": "\s*[\d\.]{4,5}" (re)
+          "start": "\s*\d+\.\d{3,4}", ? (re)
+          "time": "\s*\d+\.\d{3,4}" (re)
       }, ? (re)
       "test-skip.t": {
-          "csys": "\s*[\d\.]{4,5}", ? (re)
-          "cuser": "\s*[\d\.]{4,5}", ? (re)
+          "csys": "\s*\d+\.\d{3,4}", ? (re)
+          "cuser": "\s*\d+\.\d{3,4}", ? (re)
           "diff": "", ? (re)
-          "end": "\s*[\d\.]{4,5}", ? (re)
+          "end": "\s*\d+\.\d{3,4}", ? (re)
           "result": "skip", ? (re)
-          "start": "\s*[\d\.]{4,5}", ? (re)
-          "time": "\s*[\d\.]{4,5}" (re)
+          "start": "\s*\d+\.\d{3,4}", ? (re)
+          "time": "\s*\d+\.\d{3,4}" (re)
       }, ? (re)
       "test-success.t": [\{] (re)
-          "csys": "\s*[\d\.]{4,5}", ? (re)
-          "cuser": "\s*[\d\.]{4,5}", ? (re)
+          "csys": "\s*\d+\.\d{3,4}", ? (re)
+          "cuser": "\s*\d+\.\d{3,4}", ? (re)
           "diff": "", ? (re)
-          "end": "\s*[\d\.]{4,5}", ? (re)
+          "end": "\s*\d+\.\d{3,4}", ? (re)
           "result": "success", ? (re)
-          "start": "\s*[\d\.]{4,5}", ? (re)
-          "time": "\s*[\d\.]{4,5}" (re)
+          "start": "\s*\d+\.\d{3,4}", ? (re)
+          "time": "\s*\d+\.\d{3,4}" (re)
       }
   } (no-eol)
 --json with --outputdir
@@ -1231,31 +1231,31 @@
   $ cat output/report.json
   testreport ={
       "test-failure.t": [\{] (re)
-          "csys": "\s*[\d\.]{4,5}", ? (re)
-          "cuser": "\s*[\d\.]{4,5}", ? (re)
+          "csys": "\s*\d+\.\d{3,4}", ? (re)
+          "cuser": "\s*\d+\.\d{3,4}", ? (re)
           "diff": "---.+\+\+\+.+", ? (re)
-          "end": "\s*[\d\.]{4,5}", ? (re)
+          "end": "\s*\d+\.\d{3,4}", ? (re)
           "result": "failure", ? (re)
-          "start": "\s*[\d\.]{4,5}", ? (re)
-          "time": "\s*[\d\.]{4,5}" (re)
+          "start": "\s*\d+\.\d{3,4}", ? (re)
+          "time": "\s*\d+\.\d{3,4}" (re)
       }, ? (re)
       "test-skip.t": {
-          "csys": "\s*[\d\.]{4,5}", ? (re)
-          "cuser": "\s*[\d\.]{4,5}", ? (re)
+          "csys": "\s*\d+\.\d{3,4}", ? (re)
+          "cuser": "\s*\d+\.\d{3,4}", ? (re)
           "diff": "", ? (re)
-          "end": "\s*[\d\.]{4,5}", ? (re)
+          "end": "\s*\d+\.\d{3,4}", ? (re)
           "result": "skip", ? (re)
-          "start": "\s*[\d\.]{4,5}", ? (re)
-          "time": "\s*[\d\.]{4,5}" (re)
+          "start": "\s*\d+\.\d{3,4}", ? (re)
+          "time": "\s*\d+\.\d{3,4}" (re)
       }, ? (re)
       "test-success.t": [\{] (re)
-          "csys": "\s*[\d\.]{4,5}", ? (re)
-          "cuser": "\s*[\d\.]{4,5}", ? (re)
+          "csys": "\s*\d+\.\d{3,4}", ? (re)
+          "cuser": "\s*\d+\.\d{3,4}", ? (re)
           "diff": "", ? (re)
-          "end": "\s*[\d\.]{4,5}", ? (re)
+          "end": "\s*\d+\.\d{3,4}", ? (re)
           "result": "success", ? (re)
-          "start": "\s*[\d\.]{4,5}", ? (re)
-          "time": "\s*[\d\.]{4,5}" (re)
+          "start": "\s*\d+\.\d{3,4}", ? (re)
+          "time": "\s*\d+\.\d{3,4}" (re)
       }
   } (no-eol)
   $ ls -a output
@@ -1287,31 +1287,31 @@
   $ cat report.json
   testreport ={
       "test-failure.t": [\{] (re)
-          "csys": "\s*[\d\.]{4,5}", ? (re)
-          "cuser": "\s*[\d\.]{4,5}", ? (re)
+          "csys": "\s*\d+\.\d{3,4}", ? (re)
+          "cuser": "\s*\d+\.\d{3,4}", ? (re)
           "diff": "", ? (re)
-          "end": "\s*[\d\.]{4,5}", ? (re)
+          "end": "\s*\d+\.\d{3,4}", ? (re)
           "result": "success", ? (re)
-          "start": "\s*[\d\.]{4,5}", ? (re)
-          "time": "\s*[\d\.]{4,5}" (re)
+          "start": "\s*\d+\.\d{3,4}", ? (re)
+          "time": "\s*\d+\.\d{3,4}" (re)
       }, ? (re)
       "test-skip.t": {
-          "csys": "\s*[\d\.]{4,5}", ? (re)
-          "cuser": "\s*[\d\.]{4,5}", ? (re)
+          "csys": "\s*\d+\.\d{3,4}", ? (re)
+          "cuser": "\s*\d+\.\d{3,4}", ? (re)
           "diff": "", ? (re)
-          "end": "\s*[\d\.]{4,5}", ? (re)
+          "end": "\s*\d+\.\d{3,4}", ? (re)
           "result": "skip", ? (re)
-          "start": "\s*[\d\.]{4,5}", ? (re)
-          "time": "\s*[\d\.]{4,5}" (re)
+          "start": "\s*\d+\.\d{3,4}", ? (re)
+          "time": "\s*\d+\.\d{3,4}" (re)
       }, ? (re)
       "test-success.t": [\{] (re)
-          "csys": "\s*[\d\.]{4,5}", ? (re)
-          "cuser": "\s*[\d\.]{4,5}", ? (re)
+          "csys": "\s*\d+\.\d{3,4}", ? (re)
+          "cuser": "\s*\d+\.\d{3,4}", ? (re)
           "diff": "", ? (re)
-          "end": "\s*[\d\.]{4,5}", ? (re)
+          "end": "\s*\d+\.\d{3,4}", ? (re)
           "result": "success", ? (re)
-          "start": "\s*[\d\.]{4,5}", ? (re)
-          "time": "\s*[\d\.]{4,5}" (re)
+          "start": "\s*\d+\.\d{3,4}", ? (re)
+          "time": "\s*\d+\.\d{3,4}" (re)
       }
   } (no-eol)
   $ mv backup test-failure.t



To: lothiraldan, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

jeffpc (Jeff Sipek)
durin42 added inline comments.

INLINE COMMENTS

> test-run-tests.t:1177
>        "test-failure.t": [\{] (re)
> -          "csys": "\s*[\d\.]{4,5}", ? (re)
> -          "cuser": "\s*[\d\.]{4,5}", ? (re)
> +          "csys": "\s*\d+\.\d{3,4}", ? (re)
> +          "cuser": "\s*\d+\.\d{3,4}", ? (re)

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

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5966

To: lothiraldan, #hg-reviewers
Cc: durin42, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

jeffpc (Jeff Sipek)
In reply to this post by jeffpc (Jeff Sipek)
lothiraldan added inline comments.

INLINE COMMENTS

> durin42 wrote in test-run-tests.t:1177
> Maybe we could make it 3,5 so it's be more permissive?

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?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5966

To: lothiraldan, #hg-reviewers
Cc: durin42, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

jeffpc (Jeff Sipek)
In reply to this post by jeffpc (Jeff Sipek)
durin42 added inline comments.

INLINE COMMENTS

> lothiraldan wrote in test-run-tests.t:1177
> 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?

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

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5966

To: lothiraldan, #hg-reviewers
Cc: durin42, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

jeffpc (Jeff Sipek)
In reply to this post by jeffpc (Jeff Sipek)
lothiraldan added inline comments.

INLINE COMMENTS

> durin42 wrote in test-run-tests.t:1177
> I don't feel strongly, let's just make it permissive so the test passes on both fast and slow systems.

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.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5966

To: lothiraldan, #hg-reviewers
Cc: durin42, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

jeffpc (Jeff Sipek)
In reply to this post by jeffpc (Jeff Sipek)
durin42 added inline comments.

INLINE COMMENTS

> lothiraldan wrote in test-run-tests.t:1177
> 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.

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

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5966

To: lothiraldan, #hg-reviewers
Cc: durin42, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

jeffpc (Jeff Sipek)
In reply to this post by jeffpc (Jeff Sipek)
This revision was automatically updated to reflect the committed changes.
Closed by commit rHG4618bdf75d8a: test: stabilize test-run-tests.t output (authored by lothiraldan, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D5966?vs=14095&id=14146

REVISION DETAIL
  https://phab.mercurial-scm.org/D5966

AFFECTED FILES
  tests/test-run-tests.t

CHANGE DETAILS

diff --git a/tests/test-run-tests.t b/tests/test-run-tests.t
--- a/tests/test-run-tests.t
+++ b/tests/test-run-tests.t
@@ -1174,31 +1174,31 @@
   $ cat report.json
   testreport ={
       "test-failure.t": [\{] (re)
-          "csys": "\s*[\d\.]{4,5}", ? (re)
-          "cuser": "\s*[\d\.]{4,5}", ? (re)
+          "csys": "\s*\d+\.\d{3,4}", ? (re)
+          "cuser": "\s*\d+\.\d{3,4}", ? (re)
           "diff": "---.+\+\+\+.+", ? (re)
-          "end": "\s*[\d\.]{4,5}", ? (re)
+          "end": "\s*\d+\.\d{3,4}", ? (re)
           "result": "failure", ? (re)
-          "start": "\s*[\d\.]{4,5}", ? (re)
-          "time": "\s*[\d\.]{4,5}" (re)
+          "start": "\s*\d+\.\d{3,4}", ? (re)
+          "time": "\s*\d+\.\d{3,4}" (re)
       }, ? (re)
       "test-skip.t": {
-          "csys": "\s*[\d\.]{4,5}", ? (re)
-          "cuser": "\s*[\d\.]{4,5}", ? (re)
+          "csys": "\s*\d+\.\d{3,4}", ? (re)
+          "cuser": "\s*\d+\.\d{3,4}", ? (re)
           "diff": "", ? (re)
-          "end": "\s*[\d\.]{4,5}", ? (re)
+          "end": "\s*\d+\.\d{3,4}", ? (re)
           "result": "skip", ? (re)
-          "start": "\s*[\d\.]{4,5}", ? (re)
-          "time": "\s*[\d\.]{4,5}" (re)
+          "start": "\s*\d+\.\d{3,4}", ? (re)
+          "time": "\s*\d+\.\d{3,4}" (re)
       }, ? (re)
       "test-success.t": [\{] (re)
-          "csys": "\s*[\d\.]{4,5}", ? (re)
-          "cuser": "\s*[\d\.]{4,5}", ? (re)
+          "csys": "\s*\d+\.\d{3,4}", ? (re)
+          "cuser": "\s*\d+\.\d{3,4}", ? (re)
           "diff": "", ? (re)
-          "end": "\s*[\d\.]{4,5}", ? (re)
+          "end": "\s*\d+\.\d{3,4}", ? (re)
           "result": "success", ? (re)
-          "start": "\s*[\d\.]{4,5}", ? (re)
-          "time": "\s*[\d\.]{4,5}" (re)
+          "start": "\s*\d+\.\d{3,4}", ? (re)
+          "time": "\s*\d+\.\d{3,4}" (re)
       }
   } (no-eol)
 --json with --outputdir
@@ -1231,31 +1231,31 @@
   $ cat output/report.json
   testreport ={
       "test-failure.t": [\{] (re)
-          "csys": "\s*[\d\.]{4,5}", ? (re)
-          "cuser": "\s*[\d\.]{4,5}", ? (re)
+          "csys": "\s*\d+\.\d{3,4}", ? (re)
+          "cuser": "\s*\d+\.\d{3,4}", ? (re)
           "diff": "---.+\+\+\+.+", ? (re)
-          "end": "\s*[\d\.]{4,5}", ? (re)
+          "end": "\s*\d+\.\d{3,4}", ? (re)
           "result": "failure", ? (re)
-          "start": "\s*[\d\.]{4,5}", ? (re)
-          "time": "\s*[\d\.]{4,5}" (re)
+          "start": "\s*\d+\.\d{3,4}", ? (re)
+          "time": "\s*\d+\.\d{3,4}" (re)
       }, ? (re)
       "test-skip.t": {
-          "csys": "\s*[\d\.]{4,5}", ? (re)
-          "cuser": "\s*[\d\.]{4,5}", ? (re)
+          "csys": "\s*\d+\.\d{3,4}", ? (re)
+          "cuser": "\s*\d+\.\d{3,4}", ? (re)
           "diff": "", ? (re)
-          "end": "\s*[\d\.]{4,5}", ? (re)
+          "end": "\s*\d+\.\d{3,4}", ? (re)
           "result": "skip", ? (re)
-          "start": "\s*[\d\.]{4,5}", ? (re)
-          "time": "\s*[\d\.]{4,5}" (re)
+          "start": "\s*\d+\.\d{3,4}", ? (re)
+          "time": "\s*\d+\.\d{3,4}" (re)
       }, ? (re)
       "test-success.t": [\{] (re)
-          "csys": "\s*[\d\.]{4,5}", ? (re)
-          "cuser": "\s*[\d\.]{4,5}", ? (re)
+          "csys": "\s*\d+\.\d{3,4}", ? (re)
+          "cuser": "\s*\d+\.\d{3,4}", ? (re)
           "diff": "", ? (re)
-          "end": "\s*[\d\.]{4,5}", ? (re)
+          "end": "\s*\d+\.\d{3,4}", ? (re)
           "result": "success", ? (re)
-          "start": "\s*[\d\.]{4,5}", ? (re)
-          "time": "\s*[\d\.]{4,5}" (re)
+          "start": "\s*\d+\.\d{3,4}", ? (re)
+          "time": "\s*\d+\.\d{3,4}" (re)
       }
   } (no-eol)
   $ ls -a output
@@ -1287,31 +1287,31 @@
   $ cat report.json
   testreport ={
       "test-failure.t": [\{] (re)
-          "csys": "\s*[\d\.]{4,5}", ? (re)
-          "cuser": "\s*[\d\.]{4,5}", ? (re)
+          "csys": "\s*\d+\.\d{3,4}", ? (re)
+          "cuser": "\s*\d+\.\d{3,4}", ? (re)
           "diff": "", ? (re)
-          "end": "\s*[\d\.]{4,5}", ? (re)
+          "end": "\s*\d+\.\d{3,4}", ? (re)
           "result": "success", ? (re)
-          "start": "\s*[\d\.]{4,5}", ? (re)
-          "time": "\s*[\d\.]{4,5}" (re)
+          "start": "\s*\d+\.\d{3,4}", ? (re)
+          "time": "\s*\d+\.\d{3,4}" (re)
       }, ? (re)
       "test-skip.t": {
-          "csys": "\s*[\d\.]{4,5}", ? (re)
-          "cuser": "\s*[\d\.]{4,5}", ? (re)
+          "csys": "\s*\d+\.\d{3,4}", ? (re)
+          "cuser": "\s*\d+\.\d{3,4}", ? (re)
           "diff": "", ? (re)
-          "end": "\s*[\d\.]{4,5}", ? (re)
+          "end": "\s*\d+\.\d{3,4}", ? (re)
           "result": "skip", ? (re)
-          "start": "\s*[\d\.]{4,5}", ? (re)
-          "time": "\s*[\d\.]{4,5}" (re)
+          "start": "\s*\d+\.\d{3,4}", ? (re)
+          "time": "\s*\d+\.\d{3,4}" (re)
       }, ? (re)
       "test-success.t": [\{] (re)
-          "csys": "\s*[\d\.]{4,5}", ? (re)
-          "cuser": "\s*[\d\.]{4,5}", ? (re)
+          "csys": "\s*\d+\.\d{3,4}", ? (re)
+          "cuser": "\s*\d+\.\d{3,4}", ? (re)
           "diff": "", ? (re)
-          "end": "\s*[\d\.]{4,5}", ? (re)
+          "end": "\s*\d+\.\d{3,4}", ? (re)
           "result": "success", ? (re)
-          "start": "\s*[\d\.]{4,5}", ? (re)
-          "time": "\s*[\d\.]{4,5}" (re)
+          "start": "\s*\d+\.\d{3,4}", ? (re)
+          "time": "\s*\d+\.\d{3,4}" (re)
       }
   } (no-eol)
   $ mv backup test-failure.t



To: lothiraldan, #hg-reviewers
Cc: durin42, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel