Add :parallel-threads option to set thread pool size

[john-shaffer]

The current parallel runner uses clojure.core/map with futures, which doesn't allow much control over execution. It is probably okay for unit tests, but it uses way too many threads for integration and e2e tests. We have test systems that spawn database and browser processes, and we need a much lower level of parallelism (4 seems ideal on an 8-core machine).

This uses claypoole's pmap to run the tests, which allows for simplifying the code a bit.

[alysbrooks]

Thank you for the PR! I've been holding off on reviewing this because I'm waiting to merge the original PR. I think being able to control the thread pool somehow is a good idea. However, I'm not sure whether bringing in another dependency is worth it.

Out of curiosity, have you run the parallel threads PR against any of your test systems?

[john-shaffer]

Thanks for responding!

I considered the issue of adding a dependency, but claypoole is very light-weight and has no dependencies of its own. If that is still a problem, I could look into the possibility of vendoring just the pmap implementation. The additional control is absolutely necessary for heavy-weight test systems, both for speed and to stay within RAM constraints. It is also a useful performance-tuning knob for others.

I have been running this branch for testing locally and in CI for the last month. It has held up fine for hundreds of runs, although the increased parallelism did surface a race condition in etaoin.

[plexus]

Should we also shutdown agents if -main* exits normally?

[plexus]

👍

[plexus]

We might want to namespace this? things like type/actual/expected/line/file are standard clojure.test things, if we shove extra data into reporter events we generally namespace them. Not sure how consistent we are in this but generally a good idea.

[plexus]

Hi @john-shaffer , thanks a lot for this PR. There's some good stuff in here, and I appreciate you're helping to make Kaocha better. Sorry it's taken me so long before I managed to make time to have a better look.

I had a look at Claypoole, it has no dependencies of its own, it weighs itself about ~100kB. Also looking at what it does I think it's reasonable to bring that in here.

So, let's see if we can get this into a mergeable state? I see REPL code in source namespaces, a stray tap>, and some comments and stray whitespace that could be cleaned up. The tap> is actually causing CI to fail, let's see if we can get that all green as well. A CHANGELOG entry and where applicable a mention in the docs of new features (like the new command line flag) would be great.

Thanks a lot!

[plexus]

Hi @john-shaffer , do you still have interest in getting this in a mergable state, or was this a drive-by PR?

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (parallelize@56c1dc3). Click here to learn what that means.
The diff coverage is n/a.

@@              Coverage Diff               @@
##             parallelize     #274   +/-   ##
==============================================
  Coverage               ?   75.06%           
==============================================
  Files                  ?       51           
  Lines                  ?     2755           
  Branches               ?      256           
==============================================
  Hits                   ?     2068           
  Misses                 ?      525           
  Partials               ?      162           

Flag	Coverage Δ
integration	56.66% <0.00%> (?)
unit	69.07% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[john-shaffer]

Rebased on current parallelize branch, and updated a failing test case.