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

DEV: Add a new way to run specs in parallel with better output #7778

Merged
merged 12 commits into from
Jun 21, 2019

Conversation

danielwaterworth
Copy link
Contributor

This PR:

  1. adds a new executable, bin/interleaved_rspec which works much like
    rspec, but runs the tests in parallel.

  2. adds a rake task, rake interleaved:spec which runs the whole test
    suite.

  3. makes autospec use this new wrapper by default. You can disable this
    by running PARALLEL_SPEC=0 rake autospec.

It works much like the parallel_tests gem (and relies on it), but
makes each subprocess use a machine-readable formatter and parses this
output in order to provide a better overall summary.

(It's called interleaved, because parallel was taken and naming is
hard).

Copy link
Contributor

@eviltrout eviltrout left a comment

Choose a reason for hiding this comment

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

Overall this is excellent, good job.

I would like to be annoying though and suggest an alternative name from InterleavedTests which seems a bit technical and hard to remember/type.

How about something like:

  • FastTests
  • TurboTests
  • QuickTests
  • RapidTests

config/boot.rb Show resolved Hide resolved
bin/interleaved_rspec Outdated Show resolved Hide resolved
lib/interleaved_tests.rb Outdated Show resolved Hide resolved
lib/interleaved_tests.rb Outdated Show resolved Hide resolved
lib/interleaved_tests.rb Outdated Show resolved Hide resolved
@danielwaterworth
Copy link
Contributor Author

@eviltrout, I appreciate the comments! I will fix those things tomorrow.

@SamSaffron
Copy link
Member

Some questions:

  • Can we upstream this to the parallel test gem?

  • With @eviltrout on changing the name of the binary... maybe bin/rspec_parallel ?

  • From my perspective this is how "parallel tests" should work I am not sure how much of the interleaved naming we should carry short of internal implementation details.

  • PARALLEL_SPEC=0 rake autospec is a very strange way of enabling this. I think PARALLEL_SPEC=1 rake autospec to enable and then remove support for old parallel test integration it had, we only need this 1 pattern.

Looking forward to merging this in! Awesome work.

@danielwaterworth
Copy link
Contributor Author

  • Can we upstream this to the parallel test gem?

I've filed an issue in the parallel_tests issue tracker to discuss this.

  • With @eviltrout on changing the name of the binary... maybe bin/rspec_parallel ?

  • From my perspective this is how "parallel tests" should work I am not sure how much of the interleaved naming we should carry short of internal implementation details.

Ok

  • PARALLEL_SPEC=0 rake autospec is a very strange way of enabling this. I think PARALLEL_SPEC=1 rake autospec to enable and then remove support for old parallel test integration it had, we only need this 1 pattern.

The default is now parallel with PARALLEL_SPEC=0 being used to fallback to regular rspec. I can switch this to use SERIAL_SPEC=1 instead if you'd like.

@danielwaterworth danielwaterworth force-pushed the autospec_parallel branch 2 times, most recently from b263c5b to 436b49f Compare June 19, 2019 14:08
This commit:

 1. adds a new executable, `bin/interleaved_rspec` which works much like
    `rspec`, but runs the tests in parallel.

 2. adds a rake task, `rake interleaved:spec` which runs the whole test
    suite.

 3. makes autospec use this new wrapper by default. You can disable this
    by running `PARALLEL_SPEC=0 rake autospec`.

It works much like the `parallel_tests` gem (and relies on it), but
makes each subprocess use a machine-readable formatter and parses this
output in order to provide a better overall summary.

(It's called interleaved, because parallel was taken and naming is
hard).

command = begin
if ENV["PARALLEL_SPEC"] &&
if ENV["PARALLEL_SPEC"] != '0' &&
Copy link
Member

Choose a reason for hiding this comment

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

I prefer to have this opt in for a few weeks, so do this for now:

if ENV["PARALLEL_SPEC"] == '1'

@SamSaffron
Copy link
Member

OK ... I want to try this out... we can refine in followup commits.

@SamSaffron SamSaffron merged commit e18ce56 into discourse:master Jun 21, 2019
@danielwaterworth danielwaterworth deleted the autospec_parallel branch June 21, 2019 09:00
@discoursebot
Copy link

You've signed the CLA, danielwaterworth. Thank you! This pull request is ready for review.

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

Successfully merging this pull request may close these issues.

4 participants