From c8a87dfd47ea7db1aa4eeb4d0b2188ab2d3fda76 Mon Sep 17 00:00:00 2001 From: Agis Anastasopoulos Date: Tue, 14 Jul 2020 22:28:55 +0300 Subject: [PATCH] Add some initial tests - end-to-end tests - scheduling tests Part of #1 --- .gitignore | 1 + README.md | 31 +++++- Rakefile | 9 ++ rspecq.gemspec | 4 +- test/sample_suites/README | 1 + .../different_spec_path/mytests/foo_spec.rb | 5 + .../different_spec_path/mytests/qwe_spec.rb | 13 +++ .../failing_suite/spec/bar_spec.rb | 4 + .../failing_suite/spec/foo_spec.rb | 3 + .../flakey_suite/spec/foo_spec.rb | 8 ++ .../non_example_error/spec/bar_spec.rb | 3 + .../non_example_error/spec/foo_spec.rb | 3 + .../passing_suite/spec/foo_spec.rb | 3 + .../sample_suites/scheduling/spec/bar_spec.rb | 9 ++ .../sample_suites/scheduling/spec/foo_spec.rb | 13 +++ .../spec/bar/untimed_spec.rb | 5 + .../scheduling_untimed/spec/foo/bar_spec.rb | 6 ++ .../scheduling_untimed/spec/foo/baz_spec.rb | 5 + .../scheduling_untimed/spec/foo/foo_spec.rb | 6 ++ .../scheduling_untimed/spec/foo/zxc_spec.rb | 6 ++ .../spec_file_splitting/spec/fast_spec.rb | 4 + .../spec_file_splitting/spec/slow_spec.rb | 13 +++ test/sample_suites/timings/spec/fast_spec.rb | 6 ++ .../sample_suites/timings/spec/medium_spec.rb | 6 ++ test/sample_suites/timings/spec/slow_spec.rb | 6 ++ .../timings/spec/very_fast_spec.rb | 5 + .../timings/spec/very_slow_spec.rb | 6 ++ test/test_e2e.rb | 96 +++++++++++++++++++ test/test_helpers.rb | 70 ++++++++++++++ test/test_scheduling.rb | 78 +++++++++++++++ 30 files changed, 422 insertions(+), 6 deletions(-) create mode 100644 Rakefile create mode 100644 test/sample_suites/README create mode 100644 test/sample_suites/different_spec_path/mytests/foo_spec.rb create mode 100644 test/sample_suites/different_spec_path/mytests/qwe_spec.rb create mode 100644 test/sample_suites/failing_suite/spec/bar_spec.rb create mode 100644 test/sample_suites/failing_suite/spec/foo_spec.rb create mode 100644 test/sample_suites/flakey_suite/spec/foo_spec.rb create mode 100644 test/sample_suites/non_example_error/spec/bar_spec.rb create mode 100644 test/sample_suites/non_example_error/spec/foo_spec.rb create mode 100644 test/sample_suites/passing_suite/spec/foo_spec.rb create mode 100644 test/sample_suites/scheduling/spec/bar_spec.rb create mode 100644 test/sample_suites/scheduling/spec/foo_spec.rb create mode 100644 test/sample_suites/scheduling_untimed/spec/bar/untimed_spec.rb create mode 100644 test/sample_suites/scheduling_untimed/spec/foo/bar_spec.rb create mode 100644 test/sample_suites/scheduling_untimed/spec/foo/baz_spec.rb create mode 100644 test/sample_suites/scheduling_untimed/spec/foo/foo_spec.rb create mode 100644 test/sample_suites/scheduling_untimed/spec/foo/zxc_spec.rb create mode 100644 test/sample_suites/spec_file_splitting/spec/fast_spec.rb create mode 100644 test/sample_suites/spec_file_splitting/spec/slow_spec.rb create mode 100644 test/sample_suites/timings/spec/fast_spec.rb create mode 100644 test/sample_suites/timings/spec/medium_spec.rb create mode 100644 test/sample_suites/timings/spec/slow_spec.rb create mode 100644 test/sample_suites/timings/spec/very_fast_spec.rb create mode 100644 test/sample_suites/timings/spec/very_slow_spec.rb create mode 100644 test/test_e2e.rb create mode 100644 test/test_helpers.rb create mode 100644 test/test_scheduling.rb diff --git a/.gitignore b/.gitignore index cec3cb5..0a55386 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ *.gem +dump.rdb Gemfile.lock diff --git a/README.md b/README.md index 277b8b7..e205c60 100644 --- a/README.md +++ b/README.md @@ -89,7 +89,8 @@ in the final report (`--report`). Workers emit a timestamp after each example, as a heartbeat, to denote that they're fine and performing jobs. If a worker hasn't reported for a given amount of time (see `WORKER_LIVENESS_SEC`) it is considered dead -and the job it reserved will be requeued, so that it is picked up by another worker. +and the job it reserved will be requeued, so that it is picked up by another +worker. This protects us against unrecoverable worker failures (e.g. a segmentation fault in MRI). @@ -100,12 +101,18 @@ This protects us against unrecoverable worker failures **Update**: ci-queue [deprecated support for RSpec](https://github.com/Shopify/ci-queue/pull/149). -While evaluating ci-queue for our RSpec suite, we experienced slow worker boot times (up to 3 minutes in some cases) combined with disk saturation and increased memory consumption. This is due to the fact that a worker in ci-queue has to -load every spec file on boot. In applications with -a large number of spec files this may result in a significant performance hit and in case of cloud environments increased billings. +While evaluating ci-queue for our RSpec suite, we experienced slow worker boot +times (up to 3 minutes in some cases) combined with disk saturation and +increased memory consumption. This is due to the fact that a worker in +ci-queue has to +load every spec file on boot. In applications with large number of spec +files this may result in a significant performance hit and, in case of cloud +environments, increased usage billings. RSpecQ works with spec files as its unit of work (as opposed to ci-queue which -works with individual examples). This means that an RSpecQ worker only loads a file when it's needed and each worker only loads a subset of all files. Additionally this allows suites to keep using `before(:all)` hooks +works with individual examples). This means that an RSpecQ worker only loads a +file when it's needed and each worker only loads a subset of all files. +Additionally this allows suites to keep using `before(:all)` hooks (which ci-queue explicitly rejects). (Note: RSpecQ also schedules individual examples, but only when this is deemed necessary, see section "Spec file splitting"). @@ -121,6 +128,20 @@ file threshold" which, currently has to be set manually (but this can be improved). +## Development + +First install the required development/runtime dependencies: + +``` +$ bundle install +``` + +Then you can execute the tests after spinning up a Redis instance at +127.0.0.1:6379: + +``` +$ bundle exec rake +``` ## License diff --git a/Rakefile b/Rakefile new file mode 100644 index 0000000..ed95882 --- /dev/null +++ b/Rakefile @@ -0,0 +1,9 @@ +require "rake/testtask" + +Rake::TestTask.new do |t| + t.libs << "test" + t.test_files = FileList['test/test*.rb'] + t.verbose = true +end + +task default: :test diff --git a/rspecq.gemspec b/rspecq.gemspec index e9a90aa..41b1fe4 100644 --- a/rspecq.gemspec +++ b/rspecq.gemspec @@ -15,6 +15,8 @@ Gem::Specification.new do |s| s.add_dependency "rspec-core" s.add_dependency "redis" - s.add_development_dependency "minitest", "~> 5.14" s.add_development_dependency "rake" + s.add_development_dependency "pry-byebug" + s.add_development_dependency "minitest" + s.add_development_dependency "rspec" end diff --git a/test/sample_suites/README b/test/sample_suites/README new file mode 100644 index 0000000..64b2119 --- /dev/null +++ b/test/sample_suites/README @@ -0,0 +1 @@ +Sample RSpec suites, used as fixtures in the tests. diff --git a/test/sample_suites/different_spec_path/mytests/foo_spec.rb b/test/sample_suites/different_spec_path/mytests/foo_spec.rb new file mode 100644 index 0000000..3333d33 --- /dev/null +++ b/test/sample_suites/different_spec_path/mytests/foo_spec.rb @@ -0,0 +1,5 @@ +RSpec.describe do + it "I should not be executed!" do + expect(1).to eq 2 + end +end diff --git a/test/sample_suites/different_spec_path/mytests/qwe_spec.rb b/test/sample_suites/different_spec_path/mytests/qwe_spec.rb new file mode 100644 index 0000000..5b01dfe --- /dev/null +++ b/test/sample_suites/different_spec_path/mytests/qwe_spec.rb @@ -0,0 +1,13 @@ +RSpec.describe do + context "foo" do + describe "abc" do + it { expect(false).to be false } + end + end + + context "bar" do + describe "dfg" do + it { expect(true).to be true } + end + end +end diff --git a/test/sample_suites/failing_suite/spec/bar_spec.rb b/test/sample_suites/failing_suite/spec/bar_spec.rb new file mode 100644 index 0000000..47f8ecf --- /dev/null +++ b/test/sample_suites/failing_suite/spec/bar_spec.rb @@ -0,0 +1,4 @@ +RSpec.describe do + it { expect(false).to be false } + it { expect(1).to be 2 } +end diff --git a/test/sample_suites/failing_suite/spec/foo_spec.rb b/test/sample_suites/failing_suite/spec/foo_spec.rb new file mode 100644 index 0000000..04208a3 --- /dev/null +++ b/test/sample_suites/failing_suite/spec/foo_spec.rb @@ -0,0 +1,3 @@ +RSpec.describe do + it { expect(true).to be true } +end diff --git a/test/sample_suites/flakey_suite/spec/foo_spec.rb b/test/sample_suites/flakey_suite/spec/foo_spec.rb new file mode 100644 index 0000000..b8e0d8a --- /dev/null +++ b/test/sample_suites/flakey_suite/spec/foo_spec.rb @@ -0,0 +1,8 @@ +RSpec.describe do + it do + $tries ||= 0 + $tries += 1 + + expect($tries).to eq 3 + end +end diff --git a/test/sample_suites/non_example_error/spec/bar_spec.rb b/test/sample_suites/non_example_error/spec/bar_spec.rb new file mode 100644 index 0000000..04208a3 --- /dev/null +++ b/test/sample_suites/non_example_error/spec/bar_spec.rb @@ -0,0 +1,3 @@ +RSpec.describe do + it { expect(true).to be true } +end diff --git a/test/sample_suites/non_example_error/spec/foo_spec.rb b/test/sample_suites/non_example_error/spec/foo_spec.rb new file mode 100644 index 0000000..873b996 --- /dev/null +++ b/test/sample_suites/non_example_error/spec/foo_spec.rb @@ -0,0 +1,3 @@ +RSpec.describe IDONTEXISTZ do + it { expect(true).to be true } +end diff --git a/test/sample_suites/passing_suite/spec/foo_spec.rb b/test/sample_suites/passing_suite/spec/foo_spec.rb new file mode 100644 index 0000000..04208a3 --- /dev/null +++ b/test/sample_suites/passing_suite/spec/foo_spec.rb @@ -0,0 +1,3 @@ +RSpec.describe do + it { expect(true).to be true } +end diff --git a/test/sample_suites/scheduling/spec/bar_spec.rb b/test/sample_suites/scheduling/spec/bar_spec.rb new file mode 100644 index 0000000..318063c --- /dev/null +++ b/test/sample_suites/scheduling/spec/bar_spec.rb @@ -0,0 +1,9 @@ +RSpec.describe do + it do + expect(true).to be true + end + + it do + expect(true).to be true + end +end diff --git a/test/sample_suites/scheduling/spec/foo_spec.rb b/test/sample_suites/scheduling/spec/foo_spec.rb new file mode 100644 index 0000000..e8d3261 --- /dev/null +++ b/test/sample_suites/scheduling/spec/foo_spec.rb @@ -0,0 +1,13 @@ +RSpec.describe "slow spec file (will be split)" do + it do + sleep 0.1 + expect(true).to be true + end + + context "foo" do + it do + sleep 0.2 + expect(true).to be true + end + end +end diff --git a/test/sample_suites/scheduling_untimed/spec/bar/untimed_spec.rb b/test/sample_suites/scheduling_untimed/spec/bar/untimed_spec.rb new file mode 100644 index 0000000..d8b632c --- /dev/null +++ b/test/sample_suites/scheduling_untimed/spec/bar/untimed_spec.rb @@ -0,0 +1,5 @@ +RSpec.describe do + it do + expect(true).to be true + end +end diff --git a/test/sample_suites/scheduling_untimed/spec/foo/bar_spec.rb b/test/sample_suites/scheduling_untimed/spec/foo/bar_spec.rb new file mode 100644 index 0000000..f61f9d0 --- /dev/null +++ b/test/sample_suites/scheduling_untimed/spec/foo/bar_spec.rb @@ -0,0 +1,6 @@ +RSpec.describe do + it do + sleep 0.15 + expect(true).to be true + end +end diff --git a/test/sample_suites/scheduling_untimed/spec/foo/baz_spec.rb b/test/sample_suites/scheduling_untimed/spec/foo/baz_spec.rb new file mode 100644 index 0000000..d8b632c --- /dev/null +++ b/test/sample_suites/scheduling_untimed/spec/foo/baz_spec.rb @@ -0,0 +1,5 @@ +RSpec.describe do + it do + expect(true).to be true + end +end diff --git a/test/sample_suites/scheduling_untimed/spec/foo/foo_spec.rb b/test/sample_suites/scheduling_untimed/spec/foo/foo_spec.rb new file mode 100644 index 0000000..cb0d23b --- /dev/null +++ b/test/sample_suites/scheduling_untimed/spec/foo/foo_spec.rb @@ -0,0 +1,6 @@ +RSpec.describe do + it do + sleep 0.1 + expect(true).to be true + end +end diff --git a/test/sample_suites/scheduling_untimed/spec/foo/zxc_spec.rb b/test/sample_suites/scheduling_untimed/spec/foo/zxc_spec.rb new file mode 100644 index 0000000..cdbe47e --- /dev/null +++ b/test/sample_suites/scheduling_untimed/spec/foo/zxc_spec.rb @@ -0,0 +1,6 @@ +RSpec.describe do + it do + sleep 0.05 + expect(true).to be true + end +end diff --git a/test/sample_suites/spec_file_splitting/spec/fast_spec.rb b/test/sample_suites/spec_file_splitting/spec/fast_spec.rb new file mode 100644 index 0000000..ae8a773 --- /dev/null +++ b/test/sample_suites/spec_file_splitting/spec/fast_spec.rb @@ -0,0 +1,4 @@ +RSpec.describe do + it { expect(true).to be true } + it { expect(true).to be true } +end diff --git a/test/sample_suites/spec_file_splitting/spec/slow_spec.rb b/test/sample_suites/spec_file_splitting/spec/slow_spec.rb new file mode 100644 index 0000000..3ad6695 --- /dev/null +++ b/test/sample_suites/spec_file_splitting/spec/slow_spec.rb @@ -0,0 +1,13 @@ +RSpec.describe do + it do + sleep 0.6 + expect(true).to be true + end + + context "foo" do + it do + sleep 0.6 + expect(true).to be true + end + end +end diff --git a/test/sample_suites/timings/spec/fast_spec.rb b/test/sample_suites/timings/spec/fast_spec.rb new file mode 100644 index 0000000..cb0d23b --- /dev/null +++ b/test/sample_suites/timings/spec/fast_spec.rb @@ -0,0 +1,6 @@ +RSpec.describe do + it do + sleep 0.1 + expect(true).to be true + end +end diff --git a/test/sample_suites/timings/spec/medium_spec.rb b/test/sample_suites/timings/spec/medium_spec.rb new file mode 100644 index 0000000..61fab93 --- /dev/null +++ b/test/sample_suites/timings/spec/medium_spec.rb @@ -0,0 +1,6 @@ +RSpec.describe do + it do + sleep 0.2 + expect(true).to be true + end +end diff --git a/test/sample_suites/timings/spec/slow_spec.rb b/test/sample_suites/timings/spec/slow_spec.rb new file mode 100644 index 0000000..a49aea9 --- /dev/null +++ b/test/sample_suites/timings/spec/slow_spec.rb @@ -0,0 +1,6 @@ +RSpec.describe do + it do + sleep 0.3 + expect(true).to be true + end +end diff --git a/test/sample_suites/timings/spec/very_fast_spec.rb b/test/sample_suites/timings/spec/very_fast_spec.rb new file mode 100644 index 0000000..d8b632c --- /dev/null +++ b/test/sample_suites/timings/spec/very_fast_spec.rb @@ -0,0 +1,5 @@ +RSpec.describe do + it do + expect(true).to be true + end +end diff --git a/test/sample_suites/timings/spec/very_slow_spec.rb b/test/sample_suites/timings/spec/very_slow_spec.rb new file mode 100644 index 0000000..b63fdf4 --- /dev/null +++ b/test/sample_suites/timings/spec/very_slow_spec.rb @@ -0,0 +1,6 @@ +RSpec.describe do + it do + sleep 0.4 + expect(true).to be true + end +end diff --git a/test/test_e2e.rb b/test/test_e2e.rb new file mode 100644 index 0000000..7b21bfe --- /dev/null +++ b/test/test_e2e.rb @@ -0,0 +1,96 @@ +require "test_helpers" + +class TestEndToEnd < RSpecQTest + def test_suite_with_legit_failures + queue = exec_build("failing_suite") + + refute queue.build_successful? + + assert_processed_jobs [ + "./spec/foo_spec.rb", + "./spec/bar_spec.rb", + "./spec/bar_spec.rb[1:2]", + ], queue + + assert_equal 3+RSpecQ::MAX_REQUEUES, queue.example_count + + assert_equal({ "./spec/bar_spec.rb[1:2]" => "3" }, queue.requeued_jobs) + end + + def test_passing_suite + queue = exec_build("passing_suite") + + assert queue.build_successful? + assert_build_not_flakey(queue) + assert_equal 1, queue.example_count + assert_equal ["./spec/foo_spec.rb"], queue.processed_jobs + end + + def test_flakey_suite + queue = exec_build("flakey_suite") + + assert queue.build_successful? + assert_processed_jobs [ + "./spec/foo_spec.rb", + "./spec/foo_spec.rb[1:1]", + ], queue + + assert_equal({ "./spec/foo_spec.rb[1:1]" => "2" }, queue.requeued_jobs) + end + + def test_scheduling_by_file_and_custom_spec_path + queue = exec_build("different_spec_path", "mytests/qwe_spec.rb") + + assert queue.build_successful? + assert_build_not_flakey(queue) + assert_equal 2, queue.example_count + assert_processed_jobs ["./mytests/qwe_spec.rb"], queue + end + + def test_non_example_error + queue = exec_build("non_example_error") + + refute queue.build_successful? + assert_build_not_flakey(queue) + assert_equal 1, queue.example_count + assert_processed_jobs ["./spec/foo_spec.rb", "./spec/bar_spec.rb"], queue + assert_equal ["./spec/foo_spec.rb"], queue.non_example_errors.keys + end + + def test_timings_update + queue = exec_build("timings", "--update-timings") + + assert queue.build_successful? + + assert_equal [ + "./spec/very_fast_spec.rb", + "./spec/fast_spec.rb", + "./spec/medium_spec.rb", + "./spec/slow_spec.rb", + "./spec/very_slow_spec.rb", + ], queue.timings.sort_by { |k,v| v }.map(&:first) + end + + def test_timings_no_update + queue = exec_build("timings") + + assert queue.build_successful? + assert_empty queue.timings + end + + def test_spec_file_splitting + queue = exec_build( "spec_file_splitting", "--update-timings") + assert queue.build_successful? + refute_empty queue.timings + + queue = exec_build( "spec_file_splitting", "--file-split-threshold 1") + + assert queue.build_successful? + refute_empty queue.timings + assert_processed_jobs([ + "./spec/slow_spec.rb[1:2:1]", + "./spec/slow_spec.rb[1:1]", + "./spec/fast_spec.rb", + ], queue) + end +end diff --git a/test/test_helpers.rb b/test/test_helpers.rb new file mode 100644 index 0000000..fe11854 --- /dev/null +++ b/test/test_helpers.rb @@ -0,0 +1,70 @@ +require "minitest/autorun" +require "securerandom" +require "rspecq" +require "pry-byebug" + +module TestHelpers + REDIS_HOST = "127.0.0.1".freeze + + def rand_id + SecureRandom.hex(4) + end + + def new_worker(path) + RSpecQ::Worker.new( + build_id: rand_id, + worker_id: rand_id, + redis_host: REDIS_HOST, + files_or_dirs_to_run: suite_path(path), + ) + end + + def exec_build(path, args="") + worker_id = rand_id + build_id = rand_id + + Dir.chdir(suite_path(path)) do + out = `bundle exec rspecq --worker #{worker_id} --build #{build_id} #{args}` + puts out if ENV["RSPECQ_DEBUG"] + end + + assert_equal 0, $?.exitstatus + + queue = RSpecQ::Queue.new(build_id, worker_id, REDIS_HOST) + assert_queue_well_formed(queue) + + return queue + end + + def assert_queue_well_formed(queue, msg=nil) + redis = queue.redis + heartbeats = redis.zrange( + queue.send(:key_worker_heartbeats), 0, -1, withscores: true) + + assert queue.published? + assert queue.exhausted? + assert_operator heartbeats.size, :>=, 0 + assert heartbeats.all? { |hb| Time.at(hb.last) <= Time.now } + end + + def assert_build_not_flakey(queue) + assert_empty queue.requeued_jobs + end + + def assert_processed_jobs(exp, queue) + assert_equal exp.sort, queue.processed_jobs.sort + end + + def suite_path(path) + File.join("test", "sample_suites", path) + end +end + +# To be subclassed from all test cases. +class RSpecQTest < Minitest::Test + include TestHelpers + + def setup + Redis.new(host: REDIS_HOST).flushdb + end +end diff --git a/test/test_scheduling.rb b/test/test_scheduling.rb new file mode 100644 index 0000000..eb3c694 --- /dev/null +++ b/test/test_scheduling.rb @@ -0,0 +1,78 @@ +require "test_helpers" + +class TestScheduling < RSpecQTest + def test_scheduling_with_timings_simple + worker = new_worker("timings") + worker.populate_timings = true + worker.work + + assert_queue_well_formed(worker.queue) + + worker = new_worker("timings") + # worker.populate_timings is false by default + queue = worker.queue + worker.try_publish_queue!(queue) + + assert_equal [ + "./test/sample_suites/timings/spec/very_slow_spec.rb", + "./test/sample_suites/timings/spec/slow_spec.rb", + "./test/sample_suites/timings/spec/medium_spec.rb", + "./test/sample_suites/timings/spec/fast_spec.rb", + "./test/sample_suites/timings/spec/very_fast_spec.rb" + ], queue.unprocessed_jobs + end + + def test_scheduling_with_timings_and_splitting + worker = new_worker("scheduling") + worker.populate_timings = true + worker.work + + assert_queue_well_formed(worker.queue) + + # 1st run with timings, the slow file will be split + worker = new_worker("scheduling") + worker.populate_timings = true + worker.file_split_threshold = 0.2 + worker.work + + assert_queue_well_formed(worker.queue) + + assert_processed_jobs([ + "./test/sample_suites/scheduling/spec/bar_spec.rb", + "./test/sample_suites/scheduling/spec/foo_spec.rb[1:1]", + "./test/sample_suites/scheduling/spec/foo_spec.rb[1:2:1]", + ], worker.queue) + + # 2nd run with timings; individual example jobs will also have timings now + worker = new_worker("scheduling") + worker.populate_timings = true + worker.file_split_threshold = 0.2 + worker.try_publish_queue!(worker.queue) + + assert_equal [ + "./test/sample_suites/scheduling/spec/foo_spec.rb[1:2:1]", + "./test/sample_suites/scheduling/spec/foo_spec.rb[1:1]", + "./test/sample_suites/scheduling/spec/bar_spec.rb", + ], worker.queue.unprocessed_jobs + end + + def test_untimed_jobs_scheduled_in_the_middle + worker = new_worker("scheduling_untimed/spec/foo") + worker.populate_timings = true + worker.work + + assert_queue_well_formed(worker.queue) + assert worker.queue.build_successful? + refute_empty worker.queue.timings + + worker = new_worker("scheduling_untimed") + worker.try_publish_queue!(worker.queue) + assert_equal [ + "./test/sample_suites/scheduling_untimed/spec/foo/bar_spec.rb", + "./test/sample_suites/scheduling_untimed/spec/foo/foo_spec.rb", + "./test/sample_suites/scheduling_untimed/spec/bar/untimed_spec.rb", + "./test/sample_suites/scheduling_untimed/spec/foo/zxc_spec.rb", + "./test/sample_suites/scheduling_untimed/spec/foo/baz_spec.rb", + ], worker.queue.unprocessed_jobs + end +end