Merge pull request #137 from KnapsackPro/verify-bind-method-called

Verify test runner adapter bind method is called to track test files time execution and ensure tmp/knapsack_pro directory is not removed accidentally

CHANGELOG.md

@@ -1,5 +1,13 @@
 # Change Log
 
+### 2.11.0
+
+* Verify test runner adapter bind method is called to track test files time execution and ensure `tmp/knapsack_pro` directory is not removed accidentally
+
+    https://github.com/KnapsackPro/knapsack_pro-ruby/pull/137
+
+https://github.com/KnapsackPro/knapsack_pro-ruby/compare/v2.10.1...v2.11.0
+
 ### 2.10.1
 
 * Fix RSpec split by test examples feature broken by lazy generating of JSON report with test example ids

lib/knapsack_pro/adapters/base_adapter.rb

@@ -4,6 +4,11 @@ class BaseAdapter
       # Just example, please overwrite constant in subclass
       TEST_DIR_PATTERN = 'test/**{,/*/**}/*_test.rb'
 
+      def self.adapter_bind_method_called_file
+        adapter_name = self.to_s.gsub('::', '-')
+        "#{KnapsackPro::Config::Env::TMP_DIR}/#{adapter_name}-bind_method_called_for_node_#{KnapsackPro::Config::Env.ci_node_index}.txt"
+      end
+
       def self.slow_test_file?(adapter_class, test_file_path)
         @slow_test_file_paths ||=
           begin
@@ -26,7 +31,24 @@ def self.bind
         adapter
       end
 
+      def self.verify_bind_method_called
+        ::Kernel.at_exit do
+          if File.exists?(adapter_bind_method_called_file)
+            File.delete(adapter_bind_method_called_file)
+          else
+            puts "\n\n"
+            KnapsackPro.logger.error('-'*10 + ' Configuration error ' + '-'*50)
+            KnapsackPro.logger.error("You forgot to call #{self}.bind method in your test runner configuration file. It is needed to record test files time execution. Please follow the installation guide to configure your project properly https://docs.knapsackpro.com/knapsack_pro-ruby/guide/")
+            KnapsackPro.logger.error("If you already have #{self}.bind method added and you still see this error then one of your tests must had to delete tmp/knapsack_pro directory from the disk accidentally. Please ensure you do not remove tmp/knapsack_pro directory: https://knapsackpro.com/faq/question/why-all-test-files-have-01s-time-execution-for-my-ci-build-in-user-dashboard")
+            Kernel.exit(1)
+          end
+        end
+      end
+
       def bind
+        FileUtils.mkdir_p(KnapsackPro::Config::Env::TMP_DIR)
+        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.')
           bind_time_tracker

lib/knapsack_pro/config/env.rb

@@ -8,6 +8,7 @@ class Env
         'info'  => ::Logger::INFO,
         'debug' => ::Logger::DEBUG,
       }
+      TMP_DIR = 'tmp/knapsack_pro'
 
       class << self
         def ci_node_total

lib/knapsack_pro/report.rb

@@ -80,7 +80,7 @@ def self.create_build_subset(test_files)
 
     def self.queue_path
       queue_id = KnapsackPro::Config::Env.queue_id
-      "tmp/knapsack_pro/queue/#{queue_id}"
+      "#{KnapsackPro::Config::Env::TMP_DIR}/queue/#{queue_id}"
     end
   end
 end

lib/knapsack_pro/runners/cucumber_runner.rb

@@ -5,9 +5,12 @@ def self.run(args)
         ENV['KNAPSACK_PRO_TEST_SUITE_TOKEN'] = KnapsackPro::Config::Env.test_suite_token_cucumber
         ENV['KNAPSACK_PRO_RECORDING_ENABLED'] = 'true'
 
-        runner = new(KnapsackPro::Adapters::CucumberAdapter)
+        adapter_class = KnapsackPro::Adapters::CucumberAdapter
+        runner = new(adapter_class)
 
         if runner.test_files_to_execute_exist?
+          adapter_class.verify_bind_method_called
+
           require 'cucumber/rake/task'
 
           task_name = 'knapsack_pro:cucumber_run'

lib/knapsack_pro/runners/minitest_runner.rb

@@ -5,9 +5,12 @@ def self.run(args)
         ENV['KNAPSACK_PRO_TEST_SUITE_TOKEN'] = KnapsackPro::Config::Env.test_suite_token_minitest
         ENV['KNAPSACK_PRO_RECORDING_ENABLED'] = 'true'
 
-        runner = new(KnapsackPro::Adapters::MinitestAdapter)
+        adapter_class = KnapsackPro::Adapters::MinitestAdapter
+        runner = new(adapter_class)
 
         if runner.test_files_to_execute_exist?
+          adapter_class.verify_bind_method_called
+
           task_name = 'knapsack_pro:minitest_run'
 
           if Rake::Task.task_defined?(task_name)

lib/knapsack_pro/runners/queue/cucumber_runner.rb

@@ -39,6 +39,10 @@ def self.run_tests(accumulator)
           )
 
           if test_file_paths.empty?
+            unless all_test_file_paths.empty?
+              KnapsackPro::Adapters::CucumberAdapter.verify_bind_method_called
+            end
+
             KnapsackPro::Hooks::Queue.call_after_queue
 
             KnapsackPro::Report.save_node_queue_to_api

lib/knapsack_pro/runners/queue/minitest_runner.rb

@@ -46,6 +46,10 @@ def self.run_tests(accumulator)
           )
 
           if test_file_paths.empty?
+            unless all_test_file_paths.empty?
+              KnapsackPro::Adapters::MinitestAdapter.verify_bind_method_called
+            end
+
             KnapsackPro::Hooks::Queue.call_after_queue
 
             KnapsackPro::Report.save_node_queue_to_api

lib/knapsack_pro/runners/queue/rspec_runner.rb

@@ -53,6 +53,8 @@ def self.run_tests(accumulator)
 
           if test_file_paths.empty?
             unless all_test_file_paths.empty?
+              KnapsackPro::Adapters::RSpecAdapter.verify_bind_method_called
+
               KnapsackPro::Formatters::RSpecQueueSummaryFormatter.print_summary
               KnapsackPro::Formatters::RSpecQueueProfileFormatterExtension.print_summary
 

lib/knapsack_pro/runners/rspec_runner.rb

@@ -5,9 +5,12 @@ def self.run(args)
         ENV['KNAPSACK_PRO_TEST_SUITE_TOKEN'] = KnapsackPro::Config::Env.test_suite_token_rspec
         ENV['KNAPSACK_PRO_RECORDING_ENABLED'] = 'true'
 
-        runner = new(KnapsackPro::Adapters::RSpecAdapter)
+        adapter_class = KnapsackPro::Adapters::RSpecAdapter
+        runner = new(adapter_class)
 
         if runner.test_files_to_execute_exist?
+          adapter_class.verify_bind_method_called
+
           require 'rspec/core/rake_task'
 
           task_name = 'knapsack_pro:rspec_run'

lib/knapsack_pro/runners/spinach_runner.rb

@@ -4,9 +4,12 @@ class SpinachRunner < BaseRunner
       def self.run(args)
         ENV['KNAPSACK_PRO_TEST_SUITE_TOKEN'] = KnapsackPro::Config::Env.test_suite_token_spinach
 
-        runner = new(KnapsackPro::Adapters::SpinachAdapter)
+        adapter_class = KnapsackPro::Adapters::SpinachAdapter
+        runner = new(adapter_class)
 
         if runner.test_files_to_execute_exist?
+          adapter_class.verify_bind_method_called
+
           cmd = %Q[KNAPSACK_PRO_RECORDING_ENABLED=true KNAPSACK_PRO_TEST_SUITE_TOKEN=#{ENV['KNAPSACK_PRO_TEST_SUITE_TOKEN']} bundle exec spinach #{args} --features_path #{runner.test_dir} -- #{runner.stringify_test_file_paths}]
 
           Kernel.system(cmd)

lib/knapsack_pro/runners/test_unit_runner.rb

@@ -5,9 +5,12 @@ def self.run(args)
         ENV['KNAPSACK_PRO_TEST_SUITE_TOKEN'] = KnapsackPro::Config::Env.test_suite_token_test_unit
         ENV['KNAPSACK_PRO_RECORDING_ENABLED'] = 'true'
 
-        runner = new(KnapsackPro::Adapters::TestUnitAdapter)
+        adapter_class = KnapsackPro::Adapters::TestUnitAdapter
+        runner = new(adapter_class)
 
         if runner.test_files_to_execute_exist?
+          adapter_class.verify_bind_method_called
+
           require 'test/unit'
 
           cli_args =

lib/knapsack_pro/slow_test_file_determiner.rb

@@ -1,7 +1,7 @@
 module KnapsackPro
   class SlowTestFileDeterminer
     TIME_THRESHOLD_PER_CI_NODE = 0.7 # 70%
-    REPORT_DIR = 'tmp/knapsack_pro/slow_test_file_determiner'
+    REPORT_DIR = "#{KnapsackPro::Config::Env::TMP_DIR}/slow_test_file_determiner"
 
     # test_files: { 'path' => 'a_spec.rb', 'time_execution' => 0.0 }
     # time_execution: of build distribution (total time of CI build run)

lib/knapsack_pro/test_case_detectors/rspec_test_example_detector.rb

@@ -1,7 +1,7 @@
 module KnapsackPro
   module TestCaseDetectors
     class RSpecTestExampleDetector
-      REPORT_DIR = 'tmp/knapsack_pro/test_case_detectors/rspec'
+      REPORT_DIR = "#{KnapsackPro::Config::Env::TMP_DIR}/test_case_detectors/rspec"
 
       def generate_json_report
         require 'rspec/core'

spec/knapsack_pro/adapters/base_adapter_spec.rb

@@ -22,6 +22,27 @@
     end
   end
 
+  describe '.adapter_bind_method_called_file' do
+    subject { described_class.adapter_bind_method_called_file }
+
+    before do
+      expect(KnapsackPro::Config::Env).to receive(:ci_node_index).and_return(ci_node_index)
+    end
+
+    context 'when CI node index 0' do
+      let(:ci_node_index) { 0 }
+
+      it { expect(subject).to eq 'tmp/knapsack_pro/KnapsackPro-Adapters-BaseAdapter-bind_method_called_for_node_0.txt' }
+
+    end
+
+    context 'when CI node index 1' do
+      let(:ci_node_index) { 1 }
+
+      it { expect(subject).to eq 'tmp/knapsack_pro/KnapsackPro-Adapters-BaseAdapter-bind_method_called_for_node_1.txt' }
+    end
+  end
+
   describe '.slow_test_file?' do
     let(:adapter_class) { double }
     let(:slow_test_files) do
@@ -71,11 +92,41 @@
     it { should eql adapter }
   end
 
+  describe '.verify_bind_method_called' do
+    subject { described_class.verify_bind_method_called }
+
+    before do
+      expect(::Kernel).to receive(:at_exit).and_yield
+      expect(File).to receive(:exists?).with('tmp/knapsack_pro/KnapsackPro-Adapters-BaseAdapter-bind_method_called_for_node_0.txt').and_return(adapter_bind_method_called_file_exists)
+    end
+
+    context 'when adapter bind method called' do
+      let(:adapter_bind_method_called_file_exists) { true }
+
+      it do
+        expect(File).to receive(:delete).with('tmp/knapsack_pro/KnapsackPro-Adapters-BaseAdapter-bind_method_called_for_node_0.txt')
+        subject
+      end
+    end
+
+    context 'when adapter bind method was not call' do
+      let(:adapter_bind_method_called_file_exists) { false }
+
+      it do
+        expect(Kernel).to receive(:exit).with(1)
+        subject
+      end
+    end
+  end
+
   describe '#bind' do
     let(:recording_enabled?) { false }
     let(:queue_recording_enabled?) { false }
 
     before do
+      expect(FileUtils).to receive(:mkdir_p).with('tmp/knapsack_pro')
+      expect(File).to receive(:write).with('tmp/knapsack_pro/KnapsackPro-Adapters-BaseAdapter-bind_method_called_for_node_0.txt', nil)
+
       expect(KnapsackPro::Config::Env).to receive(:recording_enabled?).and_return(recording_enabled?)
       expect(KnapsackPro::Config::Env).to receive(:queue_recording_enabled?).and_return(queue_recording_enabled?)
     end

spec/knapsack_pro/runners/cucumber_runner_spec.rb

@@ -34,6 +34,8 @@
       let(:task) { double }
 
       before do
+        expect(KnapsackPro::Adapters::CucumberAdapter).to receive(:verify_bind_method_called)
+
         expect(Rake::Task).to receive(:[]).with('knapsack_pro:cucumber_run').at_least(1).and_return(task)
 
         t = double

spec/knapsack_pro/runners/minitest_runner_spec.rb

@@ -14,6 +14,10 @@
     end
 
     context 'when test files were returned by Knapsack Pro API' do
+      before do
+        expect(KnapsackPro::Adapters::MinitestAdapter).to receive(:verify_bind_method_called)
+      end
+
       it 'runs tests' do
         test_file_paths = ['test_fake/a_test.rb', 'test_fake/b_test.rb']
         runner = instance_double(described_class,

spec/knapsack_pro/runners/queue/cucumber_runner_spec.rb

@@ -167,14 +167,34 @@
     context "when test files don't exist" do
       let(:test_file_paths) { [] }
 
-      it 'returns exit code 0' do
-        expect(KnapsackPro::Hooks::Queue).to receive(:call_after_queue)
-        expect(KnapsackPro::Report).to receive(:save_node_queue_to_api)
+      context 'when all_test_file_paths exist' do
+        let(:all_test_file_paths) { ['features/a.feature'] }
 
-        expect(subject).to eq({
-          status: :completed,
-          exitstatus: exitstatus,
-        })
+        it 'returns exit code 0' do
+          expect(KnapsackPro::Adapters::CucumberAdapter).to receive(:verify_bind_method_called)
+
+          expect(KnapsackPro::Hooks::Queue).to receive(:call_after_queue)
+          expect(KnapsackPro::Report).to receive(:save_node_queue_to_api)
+
+          expect(subject).to eq({
+            status: :completed,
+            exitstatus: exitstatus,
+          })
+        end
+      end
+
+      context "when all_test_file_paths don't exist" do
+        let(:all_test_file_paths) { [] }
+
+        it 'returns exit code 0' do
+          expect(KnapsackPro::Hooks::Queue).to receive(:call_after_queue)
+          expect(KnapsackPro::Report).to receive(:save_node_queue_to_api)
+
+          expect(subject).to eq({
+            status: :completed,
+            exitstatus: exitstatus,
+          })
+        end
       end
     end
   end

spec/knapsack_pro/runners/queue/minitest_runner_spec.rb

@@ -159,14 +159,34 @@
     context "when test files don't exist" do
       let(:test_file_paths) { [] }
 
-      it 'returns exit code 0' do
-        expect(KnapsackPro::Hooks::Queue).to receive(:call_after_queue)
-        expect(KnapsackPro::Report).to receive(:save_node_queue_to_api)
+      context 'when all_test_file_paths exist' do
+        let(:all_test_file_paths) { ['a_test.rb'] }
 
-        expect(subject).to eq({
-          status: :completed,
-          exitstatus: exitstatus,
-        })
+        it 'returns exit code 0' do
+          expect(KnapsackPro::Adapters::MinitestAdapter).to receive(:verify_bind_method_called)
+
+          expect(KnapsackPro::Hooks::Queue).to receive(:call_after_queue)
+          expect(KnapsackPro::Report).to receive(:save_node_queue_to_api)
+
+          expect(subject).to eq({
+            status: :completed,
+            exitstatus: exitstatus,
+          })
+        end
+      end
+
+      context "when all_test_file_paths don't exist" do
+        let(:all_test_file_paths) { [] }
+
+        it 'returns exit code 0' do
+          expect(KnapsackPro::Hooks::Queue).to receive(:call_after_queue)
+          expect(KnapsackPro::Report).to receive(:save_node_queue_to_api)
+
+          expect(subject).to eq({
+            status: :completed,
+            exitstatus: exitstatus,
+          })
+        end
       end
     end
   end

spec/knapsack_pro/runners/queue/rspec_runner_spec.rb

@@ -249,6 +249,8 @@
         let(:all_test_file_paths) { ['a_spec.rb'] }
 
         it do
+          expect(KnapsackPro::Adapters::RSpecAdapter).to receive(:verify_bind_method_called)
+
           expect(KnapsackPro::Formatters::RSpecQueueSummaryFormatter).to receive(:print_summary)
           expect(KnapsackPro::Formatters::RSpecQueueProfileFormatterExtension).to receive(:print_summary)
 

spec/knapsack_pro/runners/rspec_runner_spec.rb

@@ -34,6 +34,8 @@
       let(:task) { double }
 
       before do
+        expect(KnapsackPro::Adapters::RSpecAdapter).to receive(:verify_bind_method_called)
+
         expect(Rake::Task).to receive(:[]).with('knapsack_pro:rspec_run').at_least(1).and_return(task)
 
         t = double

spec/knapsack_pro/runners/spinach_runner_spec.rb

@@ -26,6 +26,8 @@
       end
 
       before do
+        expect(KnapsackPro::Adapters::SpinachAdapter).to receive(:verify_bind_method_called)
+
         expect(Kernel).to receive(:system).with('KNAPSACK_PRO_RECORDING_ENABLED=true KNAPSACK_PRO_TEST_SUITE_TOKEN=spinach-token bundle exec spinach --custom-arg --features_path fake-test-dir -- features/a.feature features/b.feature')
       end
 

spec/knapsack_pro/runners/test_unit_runner_spec.rb

@@ -16,6 +16,8 @@
 
     context 'when test files were returned by Knapsack Pro API' do
       it 'runs tests' do
+        expect(KnapsackPro::Adapters::TestUnitAdapter).to receive(:verify_bind_method_called)
+
         test_file_paths = ['test-unit_fake/a_test.rb', 'test-unit_fake/b_test.rb']
         runner = instance_double(described_class,
                                  test_dir: 'test-unit_fake',