Pass thru test-options opts to rspec to support running with Guard (for failure-exit-code) #20

rsanheim · 2022-03-28T06:33:13Z

For #14: Allow passing custom test options via the --test-options flag so that turbo_tests works with Guard. I believe guard passes thru the custom exit code to parallel_tests so it can properly capture failure exit codes and keep rerunning things and not abort...and turbo tests needs the same thing to work with guard.

This required tweaking how the runner reports exit statuses back to the CLI -- I wasn't super sure about that piece and how it will interact with the threads, but it is reporting the correct exit code for the limited specs I've added.

I chose not to support extra options via -- for a few reasons:

I wanted to get the custom_exit_code piece working in the simplest way possible to start to support guard usage 😁
When I attempted it initially, I was hitting issues with all sorts of edge cases where you may or may not have multiple -- in the command line, and then making sure you merge in things like tags. If I had more time I would refactor the arg parsing and test it much better in isolation (probably much like parallel-tests), and then work on getting that state to the runner in a nice, clean way.

Currently all specs are passing for me locally - I thought the pending specs were failing but that was user error, I had some errant whitespace added. ✅

I also refactored the specs a bit to be easier to read with more of the state / setup close to the actual thing being tested and asserted. Hope this all makes sense to you, let me know what you think @ilyazub.

ilyazub

Thanks, @rsanheim! WDYT about support for passing options --?

lib/turbo_tests/cli.rb

ilyazub · 2022-03-29T13:52:56Z

lib/turbo_tests/cli.rb

+        opts.on("-o", "--test-options '[OPTIONS]'", "execute test commands with those options") do |arg| 
+          test_options << arg.strip
+        end


Thanks 👍 What if -- can be also used for test options similar to parallel_tests: https://github.com/grosser/parallel_tests/blob/b85a17ebd9cf920b2052623b28f0839bc35885de/lib/parallel_tests/cli.rb#L322-L339?

heya I updated the PR body w/ more context around this. tldr: I think it would be good for future PRs 😁

lib/turbo_tests/runner.rb

spec/cli_spec.rb

somesort of formatter i had went a bit crazy here

rsanheim

some notes

rsanheim · 2022-04-06T03:43:05Z

lib/turbo_tests/cli.rb

-      else
-        exit 1
-      end
+      exit exitstatus


We now just report the exitstatus that comes back from the Runner directly.

Sounds good 👍

rsanheim · 2022-04-06T03:52:47Z

lib/turbo_tests/runner.rb

+      if @reporter.failed_examples.empty? && wait_threads.map(&:value).all?(&:success?)
+        SUCCESS_EXIT_CODE
+      else
+        wait_threads.max { |value| value.exitstatus }.value.exitstatus


Hand back the highest exit code for failures -- same as parallel-tests: https://github.com/grosser/parallel_tests/blob/01218db8d43fab939fe5b53738af1fab771373dd/lib/parallel_tests/cli.rb#L90

It fails with undefined method 'exitstatus' for #<Process::Waiter:0x0000000002588318 dead> (NoMethodError) when I test it on a big test suite.

Finished in 4 minutes 38.4 seconds (files took 26.8 seconds to load) 16001 examples, 11 failures, 1 pending, 2 errors occurred outside of examples # ... bundler: failed to load command: turbo_tests (/home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/bin/turbo_tests) Traceback (most recent call last): 22: from /home/serpapi/.rbenv/versions/2.7.2/bin/bundle:25:in `<main>' 21: from /home/serpapi/.rbenv/versions/2.7.2/bin/bundle:25:in `load' 20: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/exe/bundle:37:in `<top (required)>' 19: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/lib/bundler/friendly_errors.rb:128:in `with_friendly_errors' 18: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/exe/bundle:49:in `block in <top (required)>' 17: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/lib/bundler/cli.rb:25:in `start' 16: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/lib/bundler/vendor/thor/lib/thor/base.rb:485:in `start' 15: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/lib/bundler/cli.rb:31:in `dispatch' 14: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/lib/bundler/vendor/thor/lib/thor.rb:392:in `dispatch' 13: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/lib/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command' 12: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/lib/bundler/vendor/thor/lib/thor/command.rb:27:in `run' 11: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/lib/bundler/cli.rb:477:in `exec' 10: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/lib/bundler/cli/exec.rb:23:in `run' 9: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/lib/bundler/cli/exec.rb:58:in `kernel_load' 8: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/lib/bundler/cli/exec.rb:58:in `load' 7: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/bin/turbo_tests:25:in `<top (required)>' 6: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/bin/turbo_tests:25:in `load' 5: from /home/serpapi/Workspace/turbo_tests/bin/turbo_tests:11:in `<top (required)>' 4: from /home/serpapi/Workspace/turbo_tests/lib/turbo_tests/cli.rb:101:in `run' 3: from /home/serpapi/Workspace/turbo_tests/lib/turbo_tests/runner.rb:41:in `run' 2: from /home/serpapi/Workspace/turbo_tests/lib/turbo_tests/runner.rb:107:in `run' 1: from /home/serpapi/Workspace/turbo_tests/lib/turbo_tests/runner.rb:107:in `max' /home/serpapi/Workspace/turbo_tests/lib/turbo_tests/runner.rb:107:in `block in run': undefined method `exitstatus' for #<Process::Waiter:0x0000000002588318 dead> (NoMethodError)

rsanheim · 2022-04-06T03:53:24Z

spec/cli_spec.rb

@@ -1,9 +1,62 @@
 RSpec.describe TurboTests::CLI do
-  subject(:output) { `bundle exec turbo_tests -f d #{fixture}`.strip }
+  let(:bin) { Pathname(__dir__).join("../bin/turbo_tests").expand_path }


This isn't necessary, but nice to bypass bundler overhead if we can.

Is bin used?

rsanheim · 2022-04-06T03:54:48Z

Hey @ilyazub, I have this passing for test-options and ready for another review pass. Lemme know what you think!

ilyazub

@rsanheim Cool, thanks for your work! Sorry for the late review.

For me it fails with undefined method 'exitstatus' for #<Process::Waiter:0x0000000002588318 dead> (NoMethodError) when I test it on a big test suite. I'll try to debug it too.

ilyazub · 2022-04-20T08:17:18Z

lib/turbo_tests/cli.rb

-      else
-        exit 1
-      end
+      exit exitstatus


Sounds good 👍

ilyazub · 2022-04-20T08:25:48Z

spec/cli_spec.rb

@@ -1,9 +1,62 @@
 RSpec.describe TurboTests::CLI do
-  subject(:output) { `bundle exec turbo_tests -f d #{fixture}`.strip }
+  let(:bin) { Pathname(__dir__).join("../bin/turbo_tests").expand_path }


Is bin used?

ilyazub · 2022-04-20T08:43:48Z

lib/turbo_tests/runner.rb

+      if @reporter.failed_examples.empty? && wait_threads.map(&:value).all?(&:success?)
+        SUCCESS_EXIT_CODE
+      else
+        wait_threads.max { |value| value.exitstatus }.value.exitstatus


It fails with undefined method 'exitstatus' for #<Process::Waiter:0x0000000002588318 dead> (NoMethodError) when I test it on a big test suite.

Finished in 4 minutes 38.4 seconds (files took 26.8 seconds to load) 16001 examples, 11 failures, 1 pending, 2 errors occurred outside of examples # ... bundler: failed to load command: turbo_tests (/home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/bin/turbo_tests) Traceback (most recent call last): 22: from /home/serpapi/.rbenv/versions/2.7.2/bin/bundle:25:in `<main>' 21: from /home/serpapi/.rbenv/versions/2.7.2/bin/bundle:25:in `load' 20: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/exe/bundle:37:in `<top (required)>' 19: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/lib/bundler/friendly_errors.rb:128:in `with_friendly_errors' 18: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/exe/bundle:49:in `block in <top (required)>' 17: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/lib/bundler/cli.rb:25:in `start' 16: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/lib/bundler/vendor/thor/lib/thor/base.rb:485:in `start' 15: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/lib/bundler/cli.rb:31:in `dispatch' 14: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/lib/bundler/vendor/thor/lib/thor.rb:392:in `dispatch' 13: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/lib/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command' 12: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/lib/bundler/vendor/thor/lib/thor/command.rb:27:in `run' 11: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/lib/bundler/cli.rb:477:in `exec' 10: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/lib/bundler/cli/exec.rb:23:in `run' 9: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/lib/bundler/cli/exec.rb:58:in `kernel_load' 8: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.2.27/lib/bundler/cli/exec.rb:58:in `load' 7: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/bin/turbo_tests:25:in `<top (required)>' 6: from /home/serpapi/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/bin/turbo_tests:25:in `load' 5: from /home/serpapi/Workspace/turbo_tests/bin/turbo_tests:11:in `<top (required)>' 4: from /home/serpapi/Workspace/turbo_tests/lib/turbo_tests/cli.rb:101:in `run' 3: from /home/serpapi/Workspace/turbo_tests/lib/turbo_tests/runner.rb:41:in `run' 2: from /home/serpapi/Workspace/turbo_tests/lib/turbo_tests/runner.rb:107:in `run' 1: from /home/serpapi/Workspace/turbo_tests/lib/turbo_tests/runner.rb:107:in `max' /home/serpapi/Workspace/turbo_tests/lib/turbo_tests/runner.rb:107:in `block in run': undefined method `exitstatus' for #<Process::Waiter:0x0000000002588318 dead> (NoMethodError)

john-h-k · 2023-02-10T12:40:35Z

Can I take over this work?

hartator · 2023-02-11T00:45:12Z

Can I take over this work?

Sure, PRs are always welcomed. 👍

rsanheim · 2023-02-11T19:42:37Z

Can I take over this work?

Please do, I haven't been able to carve out time to continue it and using turbo-test via guard was always a nice to have for me. I still use turbo-test every day for full build runs and it works great for that as-is.

Gloglok · 2023-03-20T15:07:46Z

Hey I'm very interested in this feature! Been using this gem on my own for a while and my team would love to use it too, but passes --test-options '--order rand' to parallel_tests.

I haven't contributed to open source before, do you think I could help finish this PR? 😄

ilyazub · 2023-03-21T16:23:55Z

@Gloglok sure, thank you!

Please also support passing test options after the end of options (--). Like in parallel_tests: https://github.com/grosser/parallel_tests/blob/7bebc55b7143afe09c4ad3d69c11fb0e15d44d77/lib/parallel_tests/cli.rb#LL310

What help is needed?

rsanheim · 2023-10-07T01:28:23Z

This is very out of date, and maybe has been handled upstream? In any case, closing this out.

Changes * Remove the default `seed` value (ref: #33) * Use exit status from RSpec calls (ref: #20)

rsanheim added 4 commits March 28, 2022 01:32

pass thru rspec options

c04d6cf

focus

a31c831

fix things

78eabbd

threading things thru

39c0ea1

ilyazub reviewed Mar 29, 2022

View reviewed changes

spec/cli_spec.rb Outdated Show resolved Hide resolved

rsanheim added 8 commits March 30, 2022 19:42

remove debugging; trying to get plain test options working

4a21f0f

format to string before hand

ea72c9b

go back to the one liner:

a6e2cf1

somesort of formatter i had went a bit crazy here

get back closer to master

70a64ff

Get test options working for custom exit code

074f68f

Remove verbose; extract constant

5ef3d50

Make sure both tags and test_options work together

089217c

Remove errant spacing

00a4baf

rsanheim marked this pull request as ready for review April 6, 2022 03:50

rsanheim changed the title ~~(draft) pass thru command line opts to rspec~~ Pass thru test-options opts to rspec to support running with Guard (for failure-exit-code) Apr 6, 2022

fix typo

48cd9de

rsanheim commented Apr 6, 2022

View reviewed changes

rsanheim requested a review from ilyazub April 6, 2022 03:54

ilyazub requested changes Apr 20, 2022

View reviewed changes

try waiting

299632a

rsanheim closed this Oct 7, 2023

rsanheim deleted the pass-thru-rspec-options branch October 7, 2023 01:28

ilyazub added a commit that referenced this pull request Nov 22, 2023

Remove default seed value

e0dc49f

Changes * Remove the default `seed` value (ref: #33) * Use exit status from RSpec calls (ref: #20)

ilyazub added a commit that referenced this pull request Nov 22, 2023

Remove default seed value

3fe0ce0

Changes * Remove the default `seed` value (ref: #33) * Use exit status from RSpec calls (ref: #20)

ilyazub mentioned this pull request Jan 17, 2024

Remove default seed value #48

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pass thru test-options opts to rspec to support running with Guard (for failure-exit-code) #20

Pass thru test-options opts to rspec to support running with Guard (for failure-exit-code) #20

rsanheim commented Mar 28, 2022 •

edited

Loading

ilyazub left a comment

ilyazub Mar 29, 2022

rsanheim Apr 6, 2022

rsanheim left a comment

rsanheim Apr 6, 2022

ilyazub Apr 20, 2022

rsanheim Apr 6, 2022

ilyazub Apr 20, 2022

rsanheim Apr 6, 2022

ilyazub Apr 20, 2022

rsanheim commented Apr 6, 2022

ilyazub left a comment

ilyazub Apr 20, 2022

ilyazub Apr 20, 2022

ilyazub Apr 20, 2022

john-h-k commented Feb 10, 2023

hartator commented Feb 11, 2023

rsanheim commented Feb 11, 2023

Gloglok commented Mar 20, 2023

ilyazub commented Mar 21, 2023

rsanheim commented Oct 7, 2023

Pass thru test-options opts to rspec to support running with Guard (for failure-exit-code) #20

Pass thru test-options opts to rspec to support running with Guard (for failure-exit-code) #20

Conversation

rsanheim commented Mar 28, 2022 • edited Loading

ilyazub left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rsanheim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rsanheim commented Apr 6, 2022

ilyazub left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

john-h-k commented Feb 10, 2023

hartator commented Feb 11, 2023

rsanheim commented Feb 11, 2023

Gloglok commented Mar 20, 2023

ilyazub commented Mar 21, 2023

rsanheim commented Oct 7, 2023

rsanheim commented Mar 28, 2022 •

edited

Loading