Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: show the progress of functional tests #14504

Merged
merged 1 commit into from Oct 24, 2018

Conversation

Projects
None yet
9 participants
@isghe
Copy link
Contributor

commented Oct 17, 2018

example: (added the progress index n/m)

1/107 - wallet_hd.py passed, Duration: 27 s
.........................................................................................
2/107 - mining_getblocktemplate_longpoll.py passed, Duration: 72 s
..................................................................
3/107 - feature_maxuploadtarget.py passed, Duration: 78 s
test/functional/test_runner.py Outdated
@@ -362,17 +362,17 @@ def run_tests(test_list, src_dir, build_dir, tmpdir, jobs=1, enable_coverage=Fal
test_results = []

max_len_name = len(max(test_list, key=len))

for _ in range(len(test_list)):
len_test_list = len(test_list)

This comment has been minimized.

Copy link
@promag

promag Oct 17, 2018

Member

nit, test_count = len(test_list)

This comment has been minimized.

Copy link
@isghe

isghe Oct 17, 2018

Author Contributor

done

@practicalswift

This comment has been minimized.

Copy link
Member

commented Oct 17, 2018

Concept ACK

@kristapsk

This comment has been minimized.

Copy link
Contributor

commented Oct 17, 2018

ACK 90829116645690c2c8b668ecc4ef117897dcb571

@fanquake fanquake added the Tests label Oct 17, 2018

@promag

This comment has been minimized.

Copy link
Member

commented Oct 18, 2018

utACK 9082911.

@fanquake fanquake requested a review from MarcoFalke Oct 18, 2018

@meshcollider

This comment has been minimized.

Copy link
Member

commented Oct 18, 2018

utACK 9082911

You should probably squash the commits though

@isghe isghe force-pushed the isghe:functional_test_progress branch to c4c95ec Oct 18, 2018

@laanwj

This comment has been minimized.

Copy link
Member

commented Oct 18, 2018

utACK, seems welcome to have a more elaborate progress display, as this script is long-running

though maybe not in all cases; there's also a valid point to be made that successful tests/compiles/etc should spam the screen/log as little as possible—for example the Duration information is duplicated in the summary table anyway!

wonder if we could do some thing where we re-use the progress line with '\r'? (replacing the ..... with a spinner, for ex.)

@laanwj

This comment has been minimized.

Copy link
Member

commented Oct 18, 2018

quick hack, but illustrates my idea:

diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index e5275bed0c17b44dd1a5cff1fd3f4f6322a9818f..6240011da9a5226c7cd6d35e1da2f47f37189a93 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -363,16 +363,17 @@ def run_tests(test_list, src_dir, build_dir, tmpdir, jobs=1, enable_coverage=Fal
     max_len_name = len(max(test_list, key=len))
     test_count = len(test_list)
+    clearline = '\r' + (' ' * 78) + '\r'
     for i in range(test_count):
         test_result, testdir, stdout, stderr = job_queue.get_next()
         test_results.append(test_result)
         if test_result.status == "Passed":
-            logging.debug("\n%s/%s - %s%s%s passed, Duration: %s s" % (i + 1, test_count, BOLD[1], test_result.name, BOLD[0], test_result.time))
+            print("%s  %s/%s - %s%s%s passed, Duration: %s s\r" % (clearline, i + 1, test_count, BOLD[1], test_result.name, BOLD[0], test_result.time), end='', flush=True)
         elif test_result.status == "Skipped":
-            logging.debug("\n%s/%s - %s%s%s skipped" % (i + 1, test_count, BOLD[1], test_result.name, BOLD[0]))
+            print("%s  %s/%s - %s%s%s skipped" % (clearline, i + 1, test_count, BOLD[1], test_result.name, BOLD[0]))
         else:
-            print("\n%s/%s - %s%s%s failed, Duration: %s s\n" % (i + 1, test_count, BOLD[1], test_result.name, BOLD[0], test_result.time))
+            print("%s  %s/%s - %s%s%s failed, Duration: %s s" % (clearline, i + 1, test_count, BOLD[1], test_result.name, BOLD[0], test_result.time))
             print(BOLD[1] + 'stdout:\n' + BOLD[0] + stdout + '\n')
             print(BOLD[1] + 'stderr:\n' + BOLD[0] + stderr + '\n')
             if combined_logs_len and os.path.isdir(testdir):
@@ -433,6 +434,8 @@ def print_results(test_results, max_len_name, runtime):
     results += "Runtime: %s s\n" % (runtime)
     print(results)
+SPINNER = '|/-\\'
+
 class TestHandler:
     """
     Trigger the test scripts passed in via the list.
@@ -471,6 +474,7 @@ class TestHandler:
                               log_stderr))
         if not self.jobs:
             raise IndexError('pop from empty list')
+        spin = 0
         while True:
             # Return first proc that finishes
             time.sleep(.5)
@@ -493,7 +497,8 @@ class TestHandler:
                     self.jobs.remove(job)
                     return TestResult(name, status, int(time.time() - start_time)), testdir, stdout, stderr
-            print('.', end='', flush=True)
+            print(SPINNER[spin] + '\r', end='', flush=True)
+            spin = (spin + 1) % len(SPINNER)
     def kill_and_join(self):
         """Send SIGKILL to all jobs and block until all have ended."""
@practicalswift

This comment has been minimized.

Copy link
Member

commented Oct 18, 2018

If we go the spinning route we'll have to make sure we spin only in cases where the spinner characters and \r don't show up in foo.txt when doing test_runner.py > foo.txt :-)

@isghe

This comment has been minimized.

Copy link
Contributor Author

commented Oct 18, 2018

@laanwj I tried to show the spinner, but the effect was a boring user experience, not seeing the progress of the current functional test isghe@4a41f8e

So now the dots are shown only for the current test in progress, "deleting" the line of dots from the previous test 2b3c35c:

$ test/functional/test_runner.py -t can_trash
Temporary test directory at can_trash/test_runner_₿_🏃_20181018_220600
1/105 - wallet_hd.py passed, Duration: 21 s
2/105 - mining_getblocktemplate_longpoll.py passed, Duration: 71 s                                 
3/105 - feature_maxuploadtarget.py passed, Duration: 68 s
..................

It also does not show the dots if --quiet arg is set cc9c616

isghe added a commit to isghe/bitcoin that referenced this pull request Oct 18, 2018

show spinner instead of dots
- removed unnecessary 'clearline'

p.s:
just an experiment from bitcoin#14504
to test user experience spinner instead of dots.
The result of user experience is negative, because watching the spinner
is *boring*, not giving the progress of the test currently running

```
$ test/functional/test_runner.py -t can_trash
Temporary test directory at can_trash/test_runner_₿_🏃_20181019_011232
1/105 - wallet_hd.py passed, Duration: 22 s
2/105 - mining_getblocktemplate_longpoll.py passed, Duration: 71 s
3/105 - feature_maxuploadtarget.py passed, Duration: 69 s
/
```

@fanquake fanquake changed the title show the progress of functional test tests: show the progress of functional tests Oct 19, 2018

@fanquake

This comment has been minimized.

Copy link
Member

commented Oct 19, 2018

@isghe please squash your commits

@isghe isghe force-pushed the isghe:functional_test_progress branch to 0d01dbf Oct 19, 2018

@isghe

This comment has been minimized.

Copy link
Contributor Author

commented Oct 20, 2018

@fanquake squashed

@fanquake

This comment has been minimized.

Copy link
Member

commented Oct 20, 2018

Here's what the output currently looks like.

test/functional/test_runner.py
Temporary test directory at /var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20181020_143946
1/106 - wallet_hd.py passed, Duration: 12 s
................
test/functional/test_runner.py
Temporary test directory at /var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20181020_143946
1/106 - wallet_hd.py passed, Duration: 12 s
2/106 - mining_getblocktemplate_longpoll.py passed, Duration: 68 s                                              
3/106 - feature_maxuploadtarget.py passed, Duration: 62 s
4/106 - wallet_backup.py passed, Duration: 106 s               
...omitted lines....
103/106 - feature_help.py passed, Duration: 0 s
104/106 - feature_config_args.py passed, Duration: 3 s
105/106 - rpc_help.py passed, Duration: 2 s
106/106 - p2p_node_network_limited.py passed, Duration: 10 s

@MarcoFalke Would be good to get your thoughts here.

else:
print("\n%s%s%s failed, Duration: %s s\n" % (BOLD[1], test_result.name, BOLD[0], test_result.time))
print("%s/%s - %s%s%s failed, Duration: %s s\n" % (i + 1, test_count, BOLD[1], test_result.name, BOLD[0], test_result.time))

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Oct 21, 2018

Member

The %s/%s - %s%s%s part is always the same. Could move it out of the branches?

done_str = "{}/{} - {}{}{}".format(i+1,test_count,BOLD...

This comment has been minimized.

Copy link
@isghe

isghe Oct 21, 2018

Author Contributor

done and squashed

@isghe isghe force-pushed the isghe:functional_test_progress branch from c01a6c9 Oct 21, 2018

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Oct 21, 2018

utACK f9d138d9a79f32106895f0d1be1b25d6bf914bcf

show the progress of functional test
example (added the progress index `n/m`)
```
1/107 - wallet_hd.py passed, Duration: 27 s
.........................................................................................
2/107 - mining_getblocktemplate_longpoll.py passed, Duration: 72 s
..................................................................
3/107 - feature_maxuploadtarget.py passed, Duration: 78 s
```

- clear dots line
```
$ test/functional/test_runner.py -t can_trash
Temporary test directory at can_trash/test_runner_₿_🏃_20181018_220600
1/105 - wallet_hd.py passed, Duration: 21 s
2/105 - mining_getblocktemplate_longpoll.py passed, Duration: 71 s
3/105 - feature_maxuploadtarget.py passed, Duration: 68 s
..................
```

- don't print the `dot` progressive if `--quiet`

- done_str
- nothing commit to check again travis tests
@conscott
Copy link
Contributor

left a comment

Tested ACK 96c509e

MarcoFalke added a commit to MarcoFalke/bitcoin that referenced this pull request Oct 24, 2018

Merge bitcoin#14504: tests: show the progress of functional tests
96c509e show the progress of functional test (Isidoro Ghezzi)

Pull request description:

  example: (added the progress index `n/m`)
  ```
  1/107 - wallet_hd.py passed, Duration: 27 s
  .........................................................................................
  2/107 - mining_getblocktemplate_longpoll.py passed, Duration: 72 s
  ..................................................................
  3/107 - feature_maxuploadtarget.py passed, Duration: 78 s
  ```

Tree-SHA512: 17b840048222e2c3676a92041b491521fee3b86049b2f2467a225aece40717732341801872d9867fcb7260e904e322c7184b76fca16d2dc687aa75dd741484ad

@MarcoFalke MarcoFalke merged commit 96c509e into bitcoin:master Oct 24, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@isghe isghe deleted the isghe:functional_test_progress branch Oct 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.