Stop calling RSpec::Core::Runner#run multiple times in Queue Mode (#…

…237)

---------

Co-authored-by: shadre <shadi.rezek@gmail.com>
Co-authored-by: Riccardo <riccardo.odone@gmail.com>

.circleci/config.yml

@@ -37,7 +37,8 @@ commands:
             fi
       - restore_cache:
           keys:
-          - v1-bundler-rails-{{ checksum "Gemfile.lock" }}-<< parameters.ruby >>
+          - v1-bundler-rails-{{ checksum "Gemfile.lock" }}-ruby-<< parameters.ruby >>-rspec-<< parameters.rspec >>
+          - v1-bundler-rails-{{ checksum "Gemfile.lock" }}-ruby-<< parameters.ruby >>-
           - v1-bundler-rails-{{ checksum "Gemfile.lock" }}-
           - v1-bundler-rails-
       - run:
@@ -48,12 +49,13 @@ commands:
       - save_cache:
           paths:
             - << parameters.path >>/vendor/bundle
-          key: v1-bundler-rails-{{ checksum "Gemfile.lock" }}-<< parameters.ruby >>
+          key: v1-bundler-rails-{{ checksum "Gemfile.lock" }}-ruby-<< parameters.ruby >>-rspec-<< parameters.rspec >>
 
 jobs:
   unit:
     parallelism: 1
     working_directory: ~/knapsack_pro-ruby
+    resource_class: small
     docker:
       - image: cimg/ruby:3.2
     steps:
@@ -63,9 +65,51 @@ jobs:
       - run: bundle exec rspec spec
       - run: bundle exec ruby spec/knapsack_pro/formatters/time_tracker_specs.rb
 
-  integration-regular-rspec:
+  integration-rspec:
+    parallelism: 1
+    working_directory: ~/knapsack_pro-ruby
+    resource_class: small
+    parameters:
+      ruby:
+        type: string
+      rspec:
+        type: string
+    docker:
+      - image: cimg/ruby:<< parameters.ruby >>
+    steps:
+      - checkout
+      - run:
+          command: |
+            if [[ "<< parameters.rspec >>" != "" ]]; then
+              sed -i 's/.*gem.*rspec-core.*/gem "rspec-core", "<< parameters.rspec >>"/g' ./Gemfile
+              echo "Updated RSpec version in Gemfile"
+            fi
+      - restore_cache:
+          keys:
+          - v1-bundler-gem-{{ checksum "knapsack_pro.gemspec" }}-ruby-<< parameters.ruby >>-rspec-<< parameters.rspec >>
+          - v1-bundler-gem-{{ checksum "knapsack_pro.gemspec" }}-ruby-<< parameters.ruby >>-
+          - v1-bundler-gem-{{ checksum "knapsack_pro.gemspec" }}-
+          - v1-bundler-gem-
+      - run:
+          command: |
+            bundle config set --local path './vendor/bundle'
+            bundle install --jobs=4 --retry=3
+      - save_cache:
+          paths:
+            - ./vendor/bundle
+          key: v1-bundler-gem-{{ checksum "knapsack_pro.gemspec" }}-ruby-<< parameters.ruby >>-rspec-<< parameters.rspec >>
+      - run:
+          command: |
+            ruby --version
+            bundle exec rspec --version
+            RSPEC=$(bundle exec rspec --version | grep rspec-core | head -n1 | cut -d " " -f5)
+            [ $RSPEC != << parameters.rspec >> ] && exit 1 || echo "Correct version of RSpec installed: $RSPEC"
+      - run: bundle exec rspec spec/integration/runners/queue/rspec_runner_spec.rb
+
+  e2e-regular-rspec:
     parallelism: 2
     working_directory: ~/knapsack_pro-ruby
+    resource_class: small
     parameters:
       ruby:
         type: string
@@ -122,15 +166,24 @@ jobs:
             export KNAPSACK_PRO_BRANCH="$CIRCLE_BRANCH--$CIRCLE_BUILD_NUM--regular--split"
             export KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES=true
             bundle exec rake knapsack_pro:rspec
+      - run:
+          working_directory: ~/rails-app-with-knapsack_pro
+          command: |
+            # split custom files by test examples ||
+            export KNAPSACK_PRO_BRANCH="$CIRCLE_BRANCH--$CIRCLE_BUILD_NUM--regular--split-custom-files"
+            export KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES=true
+            export KNAPSACK_PRO_SLOW_TEST_FILE_PATTERN="spec/features/calculator_spec.rb"
+            bundle exec rake knapsack_pro:rspec
 
-  integration-queue-rspec:
+  e2e-queue-rspec:
     parameters:
       ruby:
         type: string
       rspec:
         type: string
     parallelism: 2
     working_directory: ~/knapsack_pro-ruby
+    resource_class: small
     docker:
       - image: cimg/ruby:<< parameters.ruby >>-browsers
         environment:
@@ -216,9 +269,10 @@ jobs:
             export KNAPSACK_PRO_TEST_FILE_PATTERN="turnip/**/*.feature"
             bundle exec rake "knapsack_pro:queue:rspec[-r turnip/rspec]"
 
-  integration-regular-minitest:
+  e2e-regular-minitest:
     parallelism: 2
     working_directory: ~/knapsack_pro-ruby
+    resource_class: small
     parameters:
       ruby:
         type: string
@@ -255,12 +309,13 @@ jobs:
             export KNAPSACK_PRO_BRANCH="$CIRCLE_BRANCH--$CIRCLE_BUILD_NUM--regular"
             bundle exec rake knapsack_pro:minitest[--verbose]
 
-  integration-queue-minitest:
+  e2e-queue-minitest:
     parameters:
       ruby:
         type: string
     parallelism: 2
     working_directory: ~/knapsack_pro-ruby
+    resource_class: small
     docker:
       - image: cimg/ruby:<< parameters.ruby >>-browsers
         environment:
@@ -307,25 +362,31 @@ workflows:
   tests:
     jobs:
       - unit
-      - integration-regular-rspec:
-          name: integration-regular__ruby-<< matrix.ruby >>__rspec-<< matrix.rspec >>
+      - integration-rspec:
+          name: integration__ruby-<< matrix.ruby >>__rspec-<< matrix.rspec >>
+          matrix:
+            parameters:
+              ruby: ["3.0", "3.1", "3.2", "3.3"]
+              rspec: ["3.10.2", "3.11.0", "3.12.2"]
+      - e2e-regular-rspec:
+          name: e2e-regular__ruby-<< matrix.ruby >>__rspec-<< matrix.rspec >>
           matrix:
             parameters:
               ruby: ["3.0", "3.1", "3.2", "3.3"]
               rspec: ["3.10.2", "3.11.0", "3.12.2"]
-      - integration-queue-rspec:
-          name: integration-queue__ruby-<< matrix.ruby >>__rspec-<< matrix.rspec >>
+      - e2e-queue-rspec:
+          name: e2e-queue__ruby-<< matrix.ruby >>__rspec-<< matrix.rspec >>
           matrix:
             parameters:
               ruby: ["3.0", "3.1", "3.2", "3.3"]
               rspec: ["3.10.2", "3.11.0", "3.12.2"]
-      - integration-regular-minitest:
-          name: integration-regular__ruby-<< matrix.ruby >>__minitest
+      - e2e-regular-minitest:
+          name: e2e-regular__ruby-<< matrix.ruby >>__minitest
           matrix:
             parameters:
               ruby: ["3.0", "3.1", "3.2", "3.3"]
-      - integration-queue-minitest:
-          name: integration-queue__ruby-<< matrix.ruby >>__minitest
+      - e2e-queue-minitest:
+          name: e2e-queue__ruby-<< matrix.ruby >>__minitest
           matrix:
             parameters:
               ruby: ["3.0", "3.1", "3.2", "3.3"]

.github/pull_request_template.md

@@ -0,0 +1,22 @@
+# Story
+
+TODO: link to the internal story
+
+## Related
+
+TODO: links to related PRs or issues
+
+# Description
+
+TODO
+
+# Changes
+
+TODO: changes introduced by this PR
+
+# Checklist reminder
+
+- [ ] You follow the architecture outlined below for RSpec in Queue Mode, which is a work in progress (feel free to propose changes):
+  - Pure: `lib/knapsack_pro/pure/queue/rspec_pure.rb` contains pure functions that are unit tested.
+  - Extension: `lib/knapsack_pro/extensions/rspec_extension.rb` encapsulates calls to RSpec internals and is integration and e2e tested.
+  - Runner: `lib/knapsack_pro/runners/queue/rspec_runner.rb` invokes the pure code and the extension to produce side effects, which are integration and e2e tested.

.gitignore

@@ -34,3 +34,7 @@ Gemfile.lock
 
 # unless supporting rvm < 1.11.0 or doing something fancy, ignore this:
 .rvmrc
+
+# dynamically generated specs
+spec/knapsack_pro/formatters/time_tracker*_spec.rb
+spec_integration/

CHANGELOG.md

@@ -1,5 +1,92 @@
 # Changelog
 
+### 7.0.0
+
+* __(breaking change)__ RSpec in Queue Mode:
+  * The default for `KNAPSACK_PRO_LOG_LEVEL` is `info` instead of `debug`.
+  * The RSpec `before(:suite)` and `after(:suite)` hooks changed:
+
+    __Before:__<br>
+    The `before(:suite)` and `after(:suite)` hooks were executed multiple times. Each time for a batch of tests fetched from Knapsack Pro Queue API.
+
+    __After:__<br>
+    The `before(:suite)` and `after(:suite)` hooks are executed only once: `before(:suite)` is executed before starting tests, `after(:suite)` is executed after all tests are completed. (It is what you would expect from RSpec).
+
+  * The `KnapsackPro::Hooks::Queue.after_queue` hook change:
+
+    __Before:__<br>
+    The `KnapsackPro::Hooks::Queue.after_queue` hook is executed outside of the `after(:suite)` hook.
+
+    __After:__<br>
+    The `KnapsackPro::Hooks::Queue.after_queue` hook is executed __inside__ of the `after(:suite)` hook.
+
+* Recommended RSpec changes in your project:
+  * Remove the following code if you use Queue Mode and the `rspec_junit_formatter` gem to generate JUnit XML or JSON reports:
+
+    ```ruby
+    # REMOVE THE FOLLOWING CODE
+
+    # spec_helper.rb or rails_helper.rb
+    TMP_REPORT = "tmp/rspec_#{ENV['KNAPSACK_PRO_CI_NODE_INDEX']}.xml"
+    FINAL_REPORT = "tmp/final_rspec_#{ENV['KNAPSACK_PRO_CI_NODE_INDEX']}.xml"
+
+    KnapsackPro::Hooks::Queue.after_subset_queue do |queue_id, subset_queue_id|
+      if File.exist?(TMP_REPORT)
+        FileUtils.mv(TMP_REPORT, FINAL_REPORT)
+      end
+    end
+    ```
+
+    Learn more about [using Knapsack Pro with RSpec formatters](https://docs.knapsackpro.com/ruby/rspec/#formatters-rspec_junit_formatter-json) and [using Knapsack Pro with CircleCI](https://docs.knapsackpro.com/ruby/circleci/) in the docs.
+
+  * Replace the following code if you are using Queue Mode and the `percy-capybara` gem on a version older than 4:
+
+    Before:
+
+    ```ruby
+    KnapsackPro::Hooks::Queue.before_queue { |queue_id| Percy::Capybara.initialize_build }
+    KnapsackPro::Hooks::Queue.after_queue { |queue_id| Percy::Capybara.finalize_build }
+    ```
+
+    After:
+
+    ```ruby
+    # recommended
+    before(:suite) { Percy::Capybara.initialize_build }
+    after(:suite) { Percy::Capybara.finalize_build }
+    ```
+
+    Learn more about [using Knapsack Pro with Percy](https://docs.knapsackpro.com/ruby/hooks/#percy-capybara) in the docs.
+
+  * We are no longer modifying the default RSpec formatters in Queue Mode. You can remove the [`KNAPSACK_PRO_MODIFY_DEFAULT_RSPEC_FORMATTERS`](https://docs.knapsackpro.com/ruby/reference/#knapsack_pro_modify_default_rspec_formatters-removed-rspec) environment variable from your CI config if you are using it.
+
+* RSpec improvements in Queue Mode:
+  * Termination signals (`HUP`, `INT`, `TERM`, `ABRT`, `QUIT`, `USR1`, and `USR2`) are handled earlier: the process will terminate before the next top-level example group (`describe` or `context`) instead of waiting for the next Knapsack Pro batch of tests.
+
+  * Respect the `--error-exit-code` option. It sets a custom exit code (instead of `1`) when RSpec fails outside an example (e.g. lack of memory, termination signal).
+
+    ```bash
+    bundle exec rake "knapsack_pro:queue:rspec[--error-exit-code 3]"
+    ```
+
+  * Respect the `--failure-exit-code` option. It sets a custom exit code for when any examples fail.
+
+    ```bash
+    bundle exec rake "knapsack_pro:queue:rspec[--failure-exit-code 2]"
+    ```
+
+  * Respect the `--fail-fast` option and show a warning in the Knapsack Pro log.
+
+  * Ignore the `fail_if_no_examples` option in Queue Mode:
+    * A late CI node, started after all tests were executed by other nodes, is expected to receive an empty batch.
+    * A batch could contain tests with no examples (e.g. commented out)
+
+  * Raise an exception if the [deprecated `run_all_when_everything_filtered`](https://docs.knapsackpro.com/ruby/rspec/#some-of-my-test-files-are-not-executed) option is detected.
+
+PR with the above changes: https://github.com/KnapsackPro/knapsack_pro-ruby/pull/237
+
+https://github.com/KnapsackPro/knapsack_pro-ruby/compare/v6.0.4...v7.0.0
+
 ### 6.0.4
 
 * fix(minitest): avoid installing `at_exit` (that would result in an empty run of Minitest after Knapsack Pro is finished in Queue Mode)

Gemfile

@@ -2,3 +2,12 @@ source 'https://rubygems.org'
 
 # Specify your gem's dependencies in knapsack.gemspec
 gemspec
+
+group :test do
+  gem 'rspec_junit_formatter', require: false
+  gem 'nokogiri', require: false
+  gem 'simplecov', require: false
+
+  # This line is going to be replaced on CI to test different RSpec versions.
+  # gem 'rspec-core', 'x.x.x'
+end

README.md

@@ -42,10 +42,6 @@ The `knapsack_pro` gem supports all CIs and the following test runners:
 - Spinach
 - Turnip
 
-## Requirements
-
-`>= Ruby 2.1.0`
-
 ## Installation
 
 The [Installation Guide](https://docs.knapsackpro.com/knapsack_pro-ruby/guide/?utm_source=github&utm_medium=readme&utm_campaign=knapsack_pro-ruby_gem&utm_content=installation_guide) will ask you a few questions and generate instruction steps for your project:

knapsack_pro.gemspec

@@ -6,10 +6,11 @@ require 'knapsack_pro/version'
 Gem::Specification.new do |spec|
   spec.name          = 'knapsack_pro'
   spec.version       = KnapsackPro::VERSION
+  spec.required_ruby_version = '>= 2.7.0'
   spec.authors       = ['ArturT']
   spec.email         = ['support@knapsackpro.com']
   spec.summary       = %q{Knapsack Pro splits tests across parallel CI nodes and ensures each parallel job finish work at a similar time.}
-  spec.description   = %q{Run tests in parallel across CI server nodes based on tests execution time. Split tests in a dynamic way to ensure parallel jobs are done at a similar time. Thanks to that your CI build time is as fast as possible. It works with many CI providers.}
+  spec.description   = %q{Knapsack Pro wraps your current test runner(s) and works with your existing CI infrastructure to parallelize tests optimally. It dynamically splits your tests based on up-to-date test execution data. It's designed from the ground up for CI and supports all of them.}
   spec.homepage      = 'https://knapsackpro.com'
   spec.license       = 'MIT'
   spec.metadata    = {

lib/knapsack_pro.rb

@@ -82,6 +82,7 @@
 require_relative 'knapsack_pro/crypto/branch_encryptor'
 require_relative 'knapsack_pro/crypto/decryptor'
 require_relative 'knapsack_pro/crypto/digestor'
+require_relative 'knapsack_pro/pure/queue/rspec_pure'
 
 require 'knapsack_pro/railtie' if defined?(Rails::Railtie)
 

lib/knapsack_pro/adapters/base_adapter.rb

@@ -60,13 +60,13 @@ def bind
         File.write(self.class.adapter_bind_method_called_file, nil)
 
         if KnapsackPro::Config::Env.recording_enabled?
-          KnapsackPro.logger.debug('Test suite time execution recording enabled.')
+          KnapsackPro.logger.debug('Regular Mode enabled.')
           bind_time_tracker
           bind_save_report
         end
 
         if KnapsackPro::Config::Env.queue_recording_enabled?
-          KnapsackPro.logger.debug('Test suite time execution queue recording enabled.')
+          KnapsackPro.logger.debug('Queue Mode enabled.')
           bind_queue_mode
         end
       end
@@ -83,8 +83,13 @@ def bind_before_queue_hook
         raise NotImplementedError
       end
 
+      def bind_after_queue_hook
+        raise NotImplementedError
+      end
+
       def bind_queue_mode
         bind_before_queue_hook
+        bind_after_queue_hook
         bind_time_tracker
       end
     end

lib/knapsack_pro/adapters/cucumber_adapter.rb

@@ -65,9 +65,7 @@ def bind_before_queue_hook
         end
       end
 
-      def bind_queue_mode
-        super
-
+      def bind_after_queue_hook
         ::Kernel.at_exit do
           KnapsackPro::Hooks::Queue.call_after_subset_queue
           KnapsackPro::Report.save_subset_queue_to_file