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

support progress bar formatters #755

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ivandenysov
Copy link

What does this PR add?

new command line option --progress-bar-compatible that makes parallel_tests work nice with fuubar formatter for rspec

Why do we need it?

I tried to use fuubar with parallel_tests, but it doesn't look good: all progress bars are displayed concurrently on the same line. This PR adds an option that makes parallel_tests display every progress bar separately.

I thought that it might be possible to implement a generic solution that will work with any progress bar-like formatters in all supported test framework, but now I doubt it. I think that it will be complicated to achieve unless there will be a clear way to distinguish between the progress bar itself and other output that should be stored and displayed after all threads of parallel_tests finish execution. I want this option to act like --serialize-stdout, but it should display progress bars immediately and refresh them constantly

TODO

  • Fix blinking of progress bars. I'm probably not using the best way to rewrite content of the terminal.
    Screen-Recording-2020-03-21-at-00 21 10

  • Fix malformed output if there are pending or failing examples
    image

  • Don't use ruby's global variables for output storage and mutex

  • Add tests

Any advice or guidance will be much appreciated

@@ -0,0 +1,24 @@
require 'parallel_tests'

module ParallelTests
Copy link
Owner

@grosser grosser Mar 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some comments here would be nice (what it does / how it works etc)

@@ -212,6 +212,7 @@ def parse_options!(argv)
opts.on("--serialize-stdout", "Serialize stdout output, nothing will be written until everything is done") { options[:serialize_stdout] = true }
opts.on("--prefix-output-with-test-env-number", "Prefixes test env number to the output when not using --serialize-stdout") { options[:prefix_output_with_test_env_number] = true }
opts.on("--combine-stderr", "Combine stderr into stdout, useful in conjunction with --serialize-stdout") { options[:combine_stderr] = true }
opts.on("--progress-bar-compatible", "Serialize stdout output, but write immediately and rewrite on the fly") { options[:progress_bar_compatible] = true; options[:combine_stderr] = false }
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about store and instance of the rewriter in the options so we don't have to use global vars ?

unless options[:serialize_stdout]

if options[:progress_bar_compatible]
ParallelTests::OutputRewriter.rewrite(new_group_output: result, group_index: env['TEST_ENV_NUMBER'].to_i)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

best pass in the env number via options or a new argument

@grosser
Copy link
Owner

grosser commented Mar 21, 2020

that's some funky hackery :D
... how about make each process output via junit or so and then parse that with a single progessbar so we get an overall progress ?

@ivandenysov
Copy link
Author

@grosser Thank you for your feedback

that's some funky hackery :D
... how about make each process output via junit or so and then parse that with a single progessbar so we get an overall progress ?

Ok. I can use that approach. Maybe we can then reuse that JUnit parsing to produce a metadata file that will be used to split future test runs into more even groups (execution time wise).

@rgaufman
Copy link

rgaufman commented May 1, 2020

This looks awesome! -- How would I test this? -- I added this to my Gemfile:

  gem 'parallel_tests', github: 'ivan-denysov/parallel_tests', branch: 'support-progress-bar-formatters'

and I added --progress-bar-compatible to .rspec_parallel

Now it's saying:

$ bundle exec rake parallel:spec
/usr/local/lib/ruby/gems/2.7.0/gems/faraday-0.15.4/lib/faraday/options.rb:166: warning: Capturing the given block using Proc.new is deprecated; use `&block` instead
/usr/local/lib/ruby/gems/2.7.0/gems/faraday-0.15.4/lib/faraday/options.rb:166: warning: Capturing the given block using Proc.new is deprecated; use `&block` instead
/usr/local/lib/ruby/gems/2.7.0/gems/faraday-0.15.4/lib/faraday/options.rb:166: warning: Capturing the given block using Proc.new is deprecated; use `&block` instead
/usr/local/lib/ruby/gems/2.7.0/gems/faraday-0.15.4/lib/faraday/options.rb:166: warning: Capturing the given block using Proc.new is deprecated; use `&block` instead
/usr/local/lib/ruby/gems/2.7.0/gems/faraday-0.15.4/lib/faraday/options.rb:166: warning: Capturing the given block using Proc.new is deprecated; use `&block` instead
/Users/hackeron/Development/Tether/timeline/vendor/cache/will_paginate-66f3a4209ec5/lib/will_paginate/deprecation.rb:35: warning: Capturing the given block using Proc.new is deprecated; use `&block` instead
/Users/hackeron/Development/Tether/timeline/vendor/cache/will_paginate-66f3a4209ec5/lib/will_paginate/deprecation.rb:35: warning: Capturing the given block using Proc.new is deprecated; use `&block` instead
12 processes for 118 specs, ~ 9 specs per process
invalid option: --progress-bar-compatible (defined in .rspec_parallel)
...

Any ideas?

@mollerhoj
Copy link

mollerhoj commented Jan 10, 2022

@rgaufman you probably figured this out already, but the option goes in the terminal, not in the .rspec_parallel file:

bundle exec parallel_test spec -t rspec --progress-bar-compatible

@mollerhoj
Copy link

mollerhoj commented Jan 10, 2022

@grosser this works beautifully, and I really like it. Would you be open to this solution if fix the issues described in the TODO section?

@ivan-denysov I imagine you are no longer working on it?

It's actually quite nice to have the progress bars separated out, I gives you a good idea about how balanced the tests are.

Copy link
Owner

@grosser grosser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for this to be mergeable I'd like:

  • testing
  • docs
  • bit of cleanup (see suggestions)
  • ideally also a better name because it sounds super cryptic ... not sure if there is something better

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants