Fix log info when measured time of tests was lost

CHANGELOG.md

@@ -1,5 +1,13 @@
 # Change Log
 
+### 1.10.1
+
+* Fix log info when measured time of tests was lost
+
+    https://github.com/KnapsackPro/knapsack_pro-ruby/pull/85
+
+https://github.com/KnapsackPro/knapsack_pro-ruby/compare/v1.10.0...v1.10.1
+
 ### 1.10.0
 
 * Logs error on lost info about recorded timing for test files due to missing json files in Queue Mode

lib/knapsack_pro/report.rb

@@ -27,23 +27,28 @@ def self.save_subset_queue_to_file
       end
     end
 
-    def self.save_node_queue_to_api(executed_test_files_count)
+    def self.save_node_queue_to_api
       test_files = []
       Dir.glob("#{queue_path}/*.json").each do |file|
         report = JSON.parse(File.read(file))
         test_files += report
       end
 
       if test_files.empty?
-        if executed_test_files_count == 0
-          KnapsackPro.logger.warn("No test files were executed on this CI node.")
-          KnapsackPro.logger.debug("When you use knapsack_pro queue mode then probably reason might be that CI node was started after the test files from the queue were already executed by other CI nodes. That is why this CI node has no test files to execute.")
-          KnapsackPro.logger.debug("Another reason might be when your CI node failed in a way that prevented knapsack_pro to save time execution data to Knapsack Pro API and you have just tried to retry failed CI node but instead you got no test files to execute. In that case knapsack_pro don't know what testes should be executed here.")
-        end
+        KnapsackPro.logger.warn("No test files were executed on this CI node.")
+        KnapsackPro.logger.debug("When you use knapsack_pro queue mode then probably reason might be that CI node was started after the test files from the queue were already executed by other CI nodes. That is why this CI node has no test files to execute.")
+        KnapsackPro.logger.debug("Another reason might be when your CI node failed in a way that prevented knapsack_pro to save time execution data to Knapsack Pro API and you have just tried to retry failed CI node but instead you got no test files to execute. In that case knapsack_pro don't know what testes should be executed here.")
+      end
+
+      measured_test_files = test_files
+        .map { |t| t['time_execution'] }
+        .select { |time_execution| time_execution != KnapsackPro::Tracker::DEFAULT_TEST_FILE_TIME }
 
-        if executed_test_files_count > 0
-          KnapsackPro.logger.error("#{executed_test_files_count} test files were executed on this CI node but the recorded time of it was lost. Probably you have a code (i.e. RSpec hooks) that clears tmp directory in your project. Please ensure you do not remove the content of tmp/knapsack_pro/queue/ directory between tests run. Another reason might be that you forgot to add Knapsack::Adapters::RspecAdapter.bind in your rails_helper.rb or spec_helper.rb. Please follow the installation guide again: https://docs.knapsackpro.com/integration/")
-        end
+      if test_files.size > 0 && measured_test_files.size == 0
+        KnapsackPro.logger.warn("#{test_files.size} test files were executed on this CI node but the recorded time was lost due to:")
+        KnapsackPro.logger.warn("1. Probably you have a code (i.e. RSpec hooks) that clears tmp directory in your project. Please ensure you do not remove the content of tmp/knapsack_pro/queue/ directory between tests run.")
+        KnapsackPro.logger.warn("2. Another reason might be that you forgot to add Knapsack::Adapters::RspecAdapter.bind in your rails_helper.rb or spec_helper.rb. Please follow the installation guide again: https://docs.knapsackpro.com/integration/")
+        KnapsackPro.logger.warn("3. All your tests are empty test files, are pending tests or have syntax error and could not be executed hence no measured time execution by knapsack_pro.")
       end
 
       create_build_subset(test_files)

lib/knapsack_pro/runners/queue/minitest_runner.rb

@@ -48,7 +48,7 @@ def self.run_tests(accumulator)
           if test_file_paths.empty?
             KnapsackPro::Hooks::Queue.call_after_queue
 
-            KnapsackPro::Report.save_node_queue_to_api(all_test_file_paths.count)
+            KnapsackPro::Report.save_node_queue_to_api
 
             return {
               status: :completed,

lib/knapsack_pro/runners/queue/rspec_runner.rb

@@ -59,7 +59,7 @@ def self.run_tests(accumulator)
 
             KnapsackPro::Hooks::Queue.call_after_queue
 
-            KnapsackPro::Report.save_node_queue_to_api(all_test_file_paths.count)
+            KnapsackPro::Report.save_node_queue_to_api
 
             return {
               status: :completed,

spec/knapsack_pro/report_spec.rb

@@ -43,16 +43,14 @@
   end
 
   describe '.save_node_queue_to_api' do
-    context 'when json files with recorded time exist for executed test files' do
+    context 'when json files with recorded time does exist and test files have measured and default time execution' do
       let(:json_test_file_a_path) { double }
-      let(:json_test_file_a) { [{ 'path' => 'a_spec.rb' }] }
+      let(:json_test_file_a) { [{ 'path' => 'a_spec.rb', 'time_execution' => 0.1234 }] }
 
       let(:json_test_file_b_path) { double }
-      let(:json_test_file_b) { [{ 'path' => 'b_spec.rb' }] }
+      let(:json_test_file_b) { [{ 'path' => 'b_spec.rb', 'time_execution' => KnapsackPro::Tracker::DEFAULT_TEST_FILE_TIME }] }
 
-      let(:executed_test_files_count) { 2 }
-
-      subject { described_class.save_node_queue_to_api(executed_test_files_count) }
+      subject { described_class.save_node_queue_to_api }
 
       before do
         queue_id = 'fake-queue-id'
@@ -68,6 +66,8 @@
       end
 
       it 'creates build subset for 2 recorded test files timing' do
+        expect(KnapsackPro).not_to receive(:logger)
+
         expect(described_class).to receive(:create_build_subset).with(
           json_test_file_a + json_test_file_b
         )
@@ -76,33 +76,46 @@
       end
     end
 
-    context 'when json files with recorded time does not exist for executed test files' do
-      let(:executed_test_files_count) { 2 }
+    context 'when json files with recorded time does exist and all test files have default time execution' do
+      let(:json_test_file_a_path) { double }
+      let(:json_test_file_a) { [{ 'path' => 'a_spec.rb', 'time_execution' => KnapsackPro::Tracker::DEFAULT_TEST_FILE_TIME }] }
+
+      let(:json_test_file_b_path) { double }
+      let(:json_test_file_b) { [{ 'path' => 'b_spec.rb', 'time_execution' => KnapsackPro::Tracker::DEFAULT_TEST_FILE_TIME }] }
 
-      subject { described_class.save_node_queue_to_api(executed_test_files_count) }
+      subject { described_class.save_node_queue_to_api }
 
       before do
         queue_id = 'fake-queue-id'
         expect(KnapsackPro::Config::Env).to receive(:queue_id).and_return(queue_id)
 
-        expect(Dir).to receive(:glob).with('tmp/knapsack_pro/queue/fake-queue-id/*.json').and_return([])
+        expect(Dir).to receive(:glob).with('tmp/knapsack_pro/queue/fake-queue-id/*.json').and_return([
+          json_test_file_a_path,
+          json_test_file_b_path
+        ])
+
+        expect(File).to receive(:read).with(json_test_file_a_path).and_return(json_test_file_a.to_json)
+        expect(File).to receive(:read).with(json_test_file_b_path).and_return(json_test_file_b.to_json)
       end
 
       it 'logs error on lost info about recorded timing for test files due missing json files AND creates empty build subset' do
         logger = instance_double(Logger)
-        expect(KnapsackPro).to receive(:logger).and_return(logger)
-        expect(logger).to receive(:error).with('2 test files were executed on this CI node but the recorded time of it was lost. Probably you have a code (i.e. RSpec hooks) that clears tmp directory in your project. Please ensure you do not remove the content of tmp/knapsack_pro/queue/ directory between tests run. Another reason might be that you forgot to add Knapsack::Adapters::RspecAdapter.bind in your rails_helper.rb or spec_helper.rb. Please follow the installation guide again: https://docs.knapsackpro.com/integration/')
+        expect(KnapsackPro).to receive(:logger).exactly(4).and_return(logger)
+        expect(logger).to receive(:warn).with('2 test files were executed on this CI node but the recorded time was lost due to:')
+        expect(logger).to receive(:warn).with('1. Probably you have a code (i.e. RSpec hooks) that clears tmp directory in your project. Please ensure you do not remove the content of tmp/knapsack_pro/queue/ directory between tests run.')
+        expect(logger).to receive(:warn).with('2. Another reason might be that you forgot to add Knapsack::Adapters::RspecAdapter.bind in your rails_helper.rb or spec_helper.rb. Please follow the installation guide again: https://docs.knapsackpro.com/integration/')
+        expect(logger).to receive(:warn).with('3. All your tests are empty test files, are pending tests or have syntax error and could not be executed hence no measured time execution by knapsack_pro.')
 
-        expect(described_class).to receive(:create_build_subset).with([])
+        expect(described_class).to receive(:create_build_subset).with(
+          json_test_file_a + json_test_file_b
+        )
 
         subject
       end
     end
 
-    context 'when json files with recorded time does not exist AND no executed test files' do
-      let(:executed_test_files_count) { 0 }
-
-      subject { described_class.save_node_queue_to_api(executed_test_files_count) }
+    context 'when json files with recorded time does not exist' do
+      subject { described_class.save_node_queue_to_api }
 
       before do
         queue_id = 'fake-queue-id'

spec/knapsack_pro/runners/queue/minitest_runner_spec.rb

@@ -161,7 +161,7 @@
 
       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).with(0)
+        expect(KnapsackPro::Report).to receive(:save_node_queue_to_api)
 
         expect(subject).to eq({
           status: :completed,

spec/knapsack_pro/runners/queue/rspec_runner_spec.rb

@@ -203,7 +203,7 @@
           expect(KnapsackPro::Formatters::RSpecQueueProfileFormatterExtension).to receive(:print_summary)
 
           expect(KnapsackPro::Hooks::Queue).to receive(:call_after_queue)
-          expect(KnapsackPro::Report).to receive(:save_node_queue_to_api).with(1)
+          expect(KnapsackPro::Report).to receive(:save_node_queue_to_api)
 
           expect(subject).to eq({
             status: :completed,
@@ -217,7 +217,7 @@
 
         it do
           expect(KnapsackPro::Hooks::Queue).to receive(:call_after_queue)
-          expect(KnapsackPro::Report).to receive(:save_node_queue_to_api).with(0)
+          expect(KnapsackPro::Report).to receive(:save_node_queue_to_api)
 
           expect(subject).to eq({
             status: :completed,