Add progress trackers during test execution

[JoshKarpel]

Here's a take on https://github.com/darrenburns/ward/discussions/197 , using a Rich progress bar displayed below the running tests.

Putting this up for an early look (@darrenburns) because there are a few problems with it that might need discussion...

• Ward's own suite is so fast that you can barely see the progress bar appear, at least on my terminal 🤪 . I've taken to adding a sleep right here so that I can actually see it while playing around...
• It flickers pretty badly (at least on my terminal...).
• I don't think it is compatible with either of the dots output displays, because it seems like Progress is expecting you to not print with end="" while it's running. Perhaps we can make some improvements to Rich itself to make that work better.

[darrenburns]

This looks really cool @JoshKarpel!

One thing I did notice though is that it seems to roughly double the amount of time taken for the Ward test suite to run (I think output to the terminal is actually where Ward spends most of it's time at the moment). With the slowdown and flickering issues that seem to happen in some environments, maybe we're best putting it behind an option like --progress "bar"?

I quite like the idea of having --progress "bar" which would enable this, and a --progress "inline" which would add an extra column to the right of the default output, showing the current % progress through the suite. The reason I'd like the latter too is because I can't imagine the progress bar would play well in most CI systems.

On my laptop the Ward test suite runs in ~0.53s without the progress bar and ~1.04s with the progress bar. Running ward with --test-output-style dots-module (or dots-global), cuts the runtime down to ~0.28s.

Also, I didn't notice any flickering on my side (using iTerm2 on MacOS).

[JoshKarpel]

One thing I did notice though is that it seems to roughly double the amount of time taken for the Ward test suite to run (I think output to the terminal is actually where Ward spends most of it's time at the moment). With the slowdown and flickering issues that seem to happen in some environments, maybe we're best putting it behind an option like --progress "bar"?

I quite like the idea of having --progress "bar" which would enable this, and a --progress "inline" which would add an extra column to the right of the default output, showing the current % progress through the suite. The reason I'd like the latter too is because I can't imagine the progress bar would play well in most CI systems.

Agreed. Latest commit has inline style as well, set as the default. I actually made it possible to enable multiple styles at once by passing --progress-style multiple times (I went with --progress-style to match --test-output-style). You would pass --progress-style none to disable the default.

This might bite back later if we end up with lots of styles that are incompatible with each other, but in the meantime I like having both going at once...

On my laptop the Ward test suite runs in ~0.53s without the progress bar and ~1.04s with the progress bar. Running ward with --test-output-style dots-module (or dots-global), cuts the runtime down to ~0.28s.

Same for me, but worse :(

$ time ward --test-output-style dots-global
real    0m0.646s
user    0m0.406s
sys     0m0.219s

$ time ward
...
real    0m0.975s
user    0m0.813s
sys     0m0.156s

$ time ward --progress-style bar
real    0m3.062s
user    0m0.734s
sys     0m0.297s

Probably related to my flickering issues - seems like Windows Terminal is pretty slow.

---

I don't love how I'm passing information around right now. Some functions that were previously pretty unaware of higher-level information (configuration and test suite state) now need to know much more. I'm considering trying to roll some of this behavior back up into the TestResultWriterBase... gotta think about it more.

[codecov-io]

Codecov Report

Merging #205 (deeecda) into master (5d90387) will decrease coverage by 1.08%.
The diff coverage is 50.49%.

@@            Coverage Diff             @@
##           master     #205      +/-   ##
==========================================
- Coverage   75.62%   74.54%   -1.09%     
==========================================
  Files          17       17              
  Lines        1522     1579      +57     
  Branches      255      266      +11     
==========================================
+ Hits         1151     1177      +26     
- Misses        334      362      +28     
- Partials       37       40       +3     

Impacted Files	Coverage Δ
ward/terminal.py	47.29% <41.86%> (-1.52%)	⬇️
ward/run.py	90.21% <100.00%> (+0.44%)	⬆️
ward/suite.py	95.12% <100.00%> (+0.38%)	⬆️
ward/testing.py	93.46% <100.00%> (+0.03%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d90387...deeecda. Read the comment docs.

[JoshKarpel]

@darrenburns Well, I thought about it and didn't figure out anything clever.

For the moment this API is totally internal, so I suppose it doesn't matter much. I could imagine wanting to let people register new output styles, at which point the interface would need to get nailed down and (hopefully) shrunk. Passing the TestResultWriterBase down almost makes sense, but it does so many other things and it only saves passing a few arguments.

Let me know what you think - try to solve in this PR, or move forward with something like this and open a discussion on customizable test output? Something else?

[darrenburns]

@JoshKarpel SimpleTestResultWriter probably needs a rewrite in general, I agree it's doing a lot, and probably too much. I haven't thought about the correct approach to rewriting it, but happy to chat about it in a discussion if you have any thoughts! Deeper changes to that class are probably not in the scope of this PR though.

I've put a couple of comments on the PR, I know you still have it marked as a draft, so haven't went too deep.

[JoshKarpel]

Ok, I think this is ready for a closer look. Codecov isn't very happy with me - I added some tests of the CLI that hit the front end, but don't end up executing the full suite. This code is pretty deep and I'm not sure how to get at it to test in a structured way.

I think we could get a cheap coverage win by writing out a basic suite to a file and executing it using the click test runner, but I don't know how valuable those tests would really be 🤷🏻‍♂️ . Something to consider for an eventual SimpleTestResultWriter rewrite: good ways to drive it from the test suite!

[JoshKarpel]

Ack, this fell off my plate and I lost track of it... @darrenburns can you take another look when you get a chance?

[darrenburns]

Ack, this fell off my plate and I lost track of it... @darrenburns can you take another look when you get a chance?

Sure thing, will try and have a look at the weekend

[darrenburns]

I noticed that things can look a bit off on --test-output-style=dots-module:

(Notice the line at 97% progress - the "97%" should be aligned with the other percentages). It seems like the column containing the dots is wrapping, but haven't looked too closely yet.

P.S. Sorry for the wait on this one!

[JoshKarpel]

That was a fun one!

Ended up being a one line fix here: 7be59ca#diff-e0598ee7e5ba2e5d5466f9da72e68c3bcd19ccaed119f39beae9b66a829cad4dR351

but I tweaked a few other things while debugging and decided to keep them.

[codecov-commenter]

Codecov Report

Merging #205 (3585a99) into master (e652a47) will decrease coverage by 1.16%.
The diff coverage is 46.84%.

@@            Coverage Diff             @@
##           master     #205      +/-   ##
==========================================
- Coverage   75.62%   74.46%   -1.17%     
==========================================
  Files          17       17              
  Lines        1522     1582      +60     
  Branches      255      266      +11     
==========================================
+ Hits         1151     1178      +27     
- Misses        334      364      +30     
- Partials       37       40       +3     

Impacted Files	Coverage Δ
ward/terminal.py	47.19% <38.54%> (-1.62%)	⬇️
ward/run.py	90.21% <100.00%> (+0.44%)	⬆️
ward/suite.py	95.12% <100.00%> (+0.38%)	⬆️
ward/testing.py	93.46% <100.00%> (+0.03%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e652a47...3585a99. Read the comment docs.

[darrenburns]

Thanks Josh!

Another small issue I spotted: if the width of your terminal is exactly enough such that the dots take up the entire available width, a blank line with just the progress will be printed:

[JoshKarpel]

Thanks Josh!

Another small issue I spotted: if the width of your terminal is exactly enough such that the dots take up the entire available width, a blank line with just the progress will be printed:

Nice catch; looks like I needed to check for end-of-line before printing instead of after.

[JoshKarpel]

@darrenburns looks like an ephemeral error on one of the test runs; can you rerun CI?

How strongly do you feel about the coverage checks? I'm really not sure how to test these beyond "make sure it runs", which I suppose has some value. Would involve having a part of the test suite that writes a dummy test suite to a temp file and runs the CLI over it, I think.

[darrenburns]

@JoshKarpel Unfortunately those errors happen quite frequently in CI and there's no way to rerun a single part of a matrix-based workflow :( I've ran it again but looks like it's failed on a different version/OS combo this time.

I'm not precious about the coverage for terminal output stuff at the moment, it's an area that's always been in flux and hard to test. As part of adding plugin/hook support in #210 I'll probably be looking to rewrite a lot of terminal.py to use the Rich console protocol, and hopefully improve the testability.

[JoshKarpel]

@JoshKarpel Unfortunately those errors happen quite frequently in CI and there's no way to rerun a single part of a matrix-based workflow :( I've ran it again but looks like it's failed on a different version/OS combo this time.

:(

I'm not precious about the coverage for terminal output stuff at the moment, it's an area that's always been in flux and hard to test. As part of adding plugin/hook support in #210 I'll probably be looking to rewrite a lot of terminal.py to use the Rich console protocol, and hopefully improve the testability.

Sounds like a plan!

Anything blocking merge? Sounds like it will unblock some other work in the near future.

[darrenburns]

I just need to add some docs (there's a /docs folder in the project as of a few weeks ago that uses Sphinx and powers https://ward.readthedocs.io). I should have time to do it tomorrow, or you can add them if you like. Would just like to keep up to date with documentation as new features go out 🙂 This PR will be the next release, and hopefully can go out tomorrow.

[darrenburns]

Sorry! Thought I'd 👍 'd this earlier

[JoshKarpel]

I just need to add some docs (there's a /docs folder in the project as of a few weeks ago that uses Sphinx and powers https://ward.readthedocs.io). I should have time to do it tomorrow, or you can add them if you like. Would just like to keep up to date with documentation as new features go out 🙂 This PR will be the next release, and hopefully can go out tomorrow.

Oo, hadn't noticed this! Love it. Writing docs is tricky, and I don't want to mess with your style (or mix in screenshots that look inconsistent!), so I I think I'll let you do it, if you don't mind :)

[darrenburns]

Released as 0.55.0b0. Thanks!!!

[JoshKarpel]

Released as 0.55.0b0. Thanks!!!

My pleasure! :D