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

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

Merged
merged 24 commits into from
Jan 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
c35af5d
Verify test runner adapter bind method is called to track test files …
ArturT Jan 2, 2021
6a56e11
Update rspec_runner_spec.rb
ArturT Jan 2, 2021
ee690ec
verify bind method called for queue mode in RSpec
ArturT Jan 2, 2021
90351bf
Show 2 reasons why tests could not be recorded properly
ArturT Jan 4, 2021
f9d0d93
fix typo
ArturT Jan 4, 2021
a230079
Extract TMP_DIR
ArturT Jan 4, 2021
3a82134
Use TMP_DIR in other code places
ArturT Jan 4, 2021
9838fa6
Update CHANGELOG.md
ArturT Jan 4, 2021
ed28c7e
Update CHANGELOG.md
ArturT Jan 4, 2021
c63dc92
Update base_adapter.rb
ArturT Jan 4, 2021
d7bbfe9
verify_bind_method_called for regular mode in cucumber
ArturT Jan 4, 2021
432c7a0
Add spec to ensure KnapsackPro::Adapters::CucumberAdapter.verify_bind…
ArturT Jan 4, 2021
6be6381
Update base_adapter_spec.rb
ArturT Jan 4, 2021
1507859
Add spec for verify_bind_method_called
ArturT Jan 4, 2021
7e231ce
Explicitly exit process with 1 exit code when bind method not verifie…
ArturT Jan 4, 2021
9ed8e16
Print new lines to separate error from other messages
ArturT Jan 4, 2021
65a2f4a
verify bind method called in regular mode for Minitest
ArturT Jan 4, 2021
532be3d
Update cucumber_runner_spec.rb
ArturT Jan 4, 2021
500f60c
verify bind method called for minitest in queue mode
ArturT Jan 4, 2021
d3be702
verify bind method called for Test::Unit in regular mode
ArturT Jan 4, 2021
065d87f
verify bind method called for spinach in regular mode
ArturT Jan 4, 2021
f9623d0
Update base_adapter.rb
ArturT Jan 4, 2021
f956fe9
add node index to adapter bind method called file to allow running te…
ArturT Jan 4, 2021
7b04810
Use adapter class name in file to allow running different test runner…
ArturT Jan 4, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
22 changes: 22 additions & 0 deletions lib/knapsack_pro/adapters/base_adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions lib/knapsack_pro/config/env.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class Env
'info' => ::Logger::INFO,
'debug' => ::Logger::DEBUG,
}
TMP_DIR = 'tmp/knapsack_pro'

class << self
def ci_node_total
Expand Down
2 changes: 1 addition & 1 deletion lib/knapsack_pro/report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 4 additions & 1 deletion lib/knapsack_pro/runners/cucumber_runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
5 changes: 4 additions & 1 deletion lib/knapsack_pro/runners/minitest_runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions lib/knapsack_pro/runners/queue/cucumber_runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions lib/knapsack_pro/runners/queue/minitest_runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions lib/knapsack_pro/runners/queue/rspec_runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 4 additions & 1 deletion lib/knapsack_pro/runners/rspec_runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
5 changes: 4 additions & 1 deletion lib/knapsack_pro/runners/spinach_runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 4 additions & 1 deletion lib/knapsack_pro/runners/test_unit_runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
2 changes: 1 addition & 1 deletion lib/knapsack_pro/slow_test_file_determiner.rb
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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'
Expand Down
51 changes: 51 additions & 0 deletions spec/knapsack_pro/adapters/base_adapter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions spec/knapsack_pro/runners/cucumber_runner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions spec/knapsack_pro/runners/minitest_runner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
34 changes: 27 additions & 7 deletions spec/knapsack_pro/runners/queue/cucumber_runner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 27 additions & 7 deletions spec/knapsack_pro/runners/queue/minitest_runner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions spec/knapsack_pro/runners/queue/rspec_runner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 2 additions & 0 deletions spec/knapsack_pro/runners/rspec_runner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions spec/knapsack_pro/runners/spinach_runner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 2 additions & 0 deletions spec/knapsack_pro/runners/test_unit_runner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down