diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index 2831ce1d20e..b09842fe9a0 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -49,6 +49,14 @@ jobs: image: ${{ matrix.image }} env: POSTGRES_PASSWORD: rootpassword + # Test-only durability tuning. The data directory is tmpfs already, + # so durability has been given up — these flags just stop Postgres + # from doing the matching work. + command: >- + postgres + -c fsync=off + -c synchronous_commit=off + -c full_page_writes=off options: >- --tmpfs /var/lib/postgresql/data:rw,size=2g --health-cmd pg_isready @@ -61,8 +69,22 @@ jobs: - uses: hmarr/debug-action@v3 - uses: actions/checkout@v6 - uses: ./.github/workflows/composite/setup + - name: Restore parallel_rspec runtime logs + uses: actions/cache/restore@v4 + with: + path: tmp/parallel_runtime_rspec_*.log + key: parallel-runtime-postgres-${{ matrix.image }}-${{ github.run_id }} + restore-keys: | + parallel-runtime-postgres-${{ matrix.image }}- + parallel-runtime-postgres- - name: Run tests run: DB=postgres POSTGRES_CONNECTION_PREFIX="postgres://postgres:rootpassword@localhost:5432" bundle exec rake spec + - name: Save parallel_rspec runtime logs + if: always() + uses: actions/cache/save@v4 + with: + path: tmp/parallel_runtime_rspec_*.log + key: parallel-runtime-postgres-${{ matrix.image }}-${{ github.run_id }} - uses: ravsamhq/notify-slack-action@v2 if: github.event_name == 'push' with: @@ -84,6 +106,12 @@ jobs: env: MYSQL_DATABASE: cc_test MYSQL_ROOT_PASSWORD: password + # Test-only durability tuning. tmpfs has already removed durability, + # so skip the redo/binlog/doublewrite work that would back it up. + command: >- + --innodb-flush-log-at-trx-commit=0 + --sync-binlog=0 + --innodb-doublewrite=OFF options: >- --tmpfs /var/lib/mysql:rw,size=2g --health-cmd="mysqladmin ping" @@ -96,8 +124,22 @@ jobs: - uses: hmarr/debug-action@v3 - uses: actions/checkout@v6 - uses: ./.github/workflows/composite/setup + - name: Restore parallel_rspec runtime logs + uses: actions/cache/restore@v4 + with: + path: tmp/parallel_runtime_rspec_*.log + key: parallel-runtime-mysql-${{ matrix.image }}-${{ github.run_id }} + restore-keys: | + parallel-runtime-mysql-${{ matrix.image }}- + parallel-runtime-mysql- - name: Run tests run: DB=mysql MYSQL_CONNECTION_PREFIX="mysql2://root:password@127.0.0.1:3306" bundle exec rake spec + - name: Save parallel_rspec runtime logs + if: always() + uses: actions/cache/save@v4 + with: + path: tmp/parallel_runtime_rspec_*.log + key: parallel-runtime-mysql-${{ matrix.image }}-${{ github.run_id }} - uses: ravsamhq/notify-slack-action@v2 if: github.event_name == 'push' with: diff --git a/.rspec_parallel b/.rspec_parallel index e1e55ab2d80..58f17d045ba 100644 --- a/.rspec_parallel +++ b/.rspec_parallel @@ -2,5 +2,4 @@ --format RSpec::Instafail --format progress --format ParallelTests::RSpec::SummaryLogger --out tmp/spec_summary.log ---format ParallelTests::RSpec::RuntimeLogger --out tmp/parallel_runtime_rspec.log --order rand diff --git a/.rubocop_cc.yml b/.rubocop_cc.yml index 1d7b2bd8aba..ff096a0bf17 100644 --- a/.rubocop_cc.yml +++ b/.rubocop_cc.yml @@ -47,6 +47,7 @@ Metrics/BlockLength: - config/routes.rb - lib/tasks/db.rake - lib/tasks/jobs.rake + - lib/tasks/spec.rake Max: 50 Metrics/CyclomaticComplexity: Max: 12 diff --git a/lib/tasks/spec.rake b/lib/tasks/spec.rake index 06b8e5923c3..368b1a8148c 100644 --- a/lib/tasks/spec.rake +++ b/lib/tasks/spec.rake @@ -7,7 +7,7 @@ namespace :spec do run_specs(ARGV[1]) else run_specs_parallel('spec') - # Run isolated specs separately since they might affect other tests + run_migration_specs_parallel run_specs('spec/isolated_specs') end end @@ -32,6 +32,7 @@ namespace :spec do run_specs(ARGV[1], 'NO_DB_MIGRATION=true') else run_specs_parallel('spec', 'NO_DB_MIGRATION=true') + run_migration_specs_parallel('NO_DB_MIGRATION=true') # Run isolated specs separately since they might affect other tests run_specs('spec/isolated_specs', 'NO_DB_MIGRATION=true') end @@ -44,17 +45,29 @@ namespace :spec do def run_specs_parallel(path, env_vars='') command = <<~CMD #{env_vars} bundle exec parallel_rspec \ - --test-options '--order rand' \ + --test-options '--order rand --profile 20 --format ParallelTests::RSpec::RuntimeLogger --out tmp/parallel_runtime_rspec_main.log' \ + --runtime-log tmp/parallel_runtime_rspec_main.log \ --single spec/integration/ \ --single spec/acceptance/ \ --isolate \ - --exclude-pattern 'spec/isolated_specs/' \ + --exclude-pattern '(spec/isolated_specs/|spec/migrations/)' \ -- #{path} CMD sh command end + def run_migration_specs_parallel(env_vars='') + command = <<~CMD + #{env_vars} bundle exec parallel_rspec \ + --test-options '--order rand --profile 20 --format ParallelTests::RSpec::RuntimeLogger --out tmp/parallel_runtime_rspec_migrations.log' \ + --runtime-log tmp/parallel_runtime_rspec_migrations.log \ + -- spec/migrations + CMD + + sh command + end + def run_failed_specs sh 'bundle exec rspec --only-failures --color --tty spec --require rspec/instafail --format RSpec::Instafail' end diff --git a/spec/migrations/helpers/bigint_migration_step1_shared_context.rb b/spec/migrations/helpers/bigint_migration_step1_shared_context.rb index cc85de3c8f7..81b48f484a7 100644 --- a/spec/migrations/helpers/bigint_migration_step1_shared_context.rb +++ b/spec/migrations/helpers/bigint_migration_step1_shared_context.rb @@ -3,13 +3,13 @@ require 'database/bigint_migration' RSpec.shared_context 'bigint migration step1' do + before(:all) { skip unless Sequel::Model.db.database_type == :postgres } # rubocop:disable RSpec/BeforeAfterAll + include_context 'migration' let(:skip_bigint_id_migration) { nil } before do - skip unless db.database_type == :postgres - allow_any_instance_of(VCAP::CloudController::Config).to receive(:get).with(:skip_bigint_id_migration).and_return(skip_bigint_id_migration) allow_any_instance_of(VCAP::CloudController::Config).to receive(:get).with(:migration_psql_concurrent_statement_timeout_in_seconds).and_return(300) end diff --git a/spec/migrations/helpers/bigint_migration_step3_shared_context.rb b/spec/migrations/helpers/bigint_migration_step3_shared_context.rb index 3efb43f5c74..2378c849909 100644 --- a/spec/migrations/helpers/bigint_migration_step3_shared_context.rb +++ b/spec/migrations/helpers/bigint_migration_step3_shared_context.rb @@ -3,6 +3,8 @@ require 'database/bigint_migration' RSpec.shared_context 'bigint migration step3a' do + before(:all) { skip unless Sequel::Model.db.database_type == :postgres } # rubocop:disable RSpec/BeforeAfterAll + let(:migration_filename) { migration_filename_step1 } let(:current_migration_index_step3a) { migration_filename_step3a.match(/\A\d+/)[0].to_i } @@ -12,8 +14,6 @@ let(:logger) { double(:logger, info: nil) } before do - skip unless db.database_type == :postgres - allow_any_instance_of(VCAP::CloudController::Config).to receive(:get).with(:skip_bigint_id_migration).and_return(skip_bigint_id_migration) allow_any_instance_of(VCAP::CloudController::Config).to receive(:get).with(:migration_psql_concurrent_statement_timeout_in_seconds).and_return(300) end @@ -107,6 +107,8 @@ end RSpec.shared_context 'bigint migration step3b' do + before(:all) { skip unless Sequel::Model.db.database_type == :postgres } # rubocop:disable RSpec/BeforeAfterAll + let(:migration_filename) { migration_filename_step1 } let(:current_migration_index_step3a) { migration_filename_step3a.match(/\A\d+/)[0].to_i } let(:current_migration_index_step3b) { migration_filename_step3b.match(/\A\d+/)[0].to_i } @@ -117,8 +119,6 @@ let(:logger) { double(:logger, info: nil) } before do - skip unless db.database_type == :postgres - allow_any_instance_of(VCAP::CloudController::Config).to receive(:get).with(:skip_bigint_id_migration).and_return(skip_bigint_id_migration) allow_any_instance_of(VCAP::CloudController::Config).to receive(:get).with(:migration_psql_concurrent_statement_timeout_in_seconds).and_return(300) end diff --git a/spec/support/database_isolation.rb b/spec/support/database_isolation.rb index e8bbab49e62..e8f60ab0be6 100644 --- a/spec/support/database_isolation.rb +++ b/spec/support/database_isolation.rb @@ -8,20 +8,49 @@ def self.choose(isolation, db) end end + # Sequel logger that captures the names of tables written to (INSERT/UPDATE/DELETE). + # Attached to the DB only for the duration of one example so we can truncate just + # the tables the example dirtied, instead of all 100+ tables. + class WrittenTablesLogger + WRITE_REGEX = /\b(?:INSERT INTO|UPDATE|DELETE FROM|TRUNCATE TABLE|TRUNCATE)\s+[`"]?(\w+)/i + + attr_reader :tables + + def initialize + @tables = Set.new + end + + def capture(msg) + @tables << ::Regexp.last_match(1).to_sym if msg =~ WRITE_REGEX + end + + alias_method :info, :capture + alias_method :warn, :capture + alias_method :debug, :capture + alias_method :error, :capture + alias_method :fatal, :capture + end + class TruncateTables def initialize(db) @db = db end def cleanly - yield - ensure - reset_tables + logger = WrittenTablesLogger.new + db.loggers << logger + begin + yield + ensure + db.loggers.delete(logger) + reset_tables(logger.tables.to_a & TableTruncator.isolated_tables(db)) + end end - def reset_tables - table_truncator = TableTruncator.new(db) - table_truncator.truncate_tables + def reset_tables(tables) + return if tables.empty? + + TableTruncator.new(db, tables).truncate_tables # VCAP::CloudController::Seeds requires the :api config TestConfig.context = :api diff --git a/spec/unit/jobs/runtime/prune_completed_builds_spec.rb b/spec/unit/jobs/runtime/prune_completed_builds_spec.rb index 4d64bbff151..5ed9cbc5942 100644 --- a/spec/unit/jobs/runtime/prune_completed_builds_spec.rb +++ b/spec/unit/jobs/runtime/prune_completed_builds_spec.rb @@ -3,7 +3,7 @@ module VCAP::CloudController module Jobs::Runtime RSpec.describe PruneCompletedBuilds, job_context: :worker do - let(:max_retained_builds_per_app) { 15 } + let(:max_retained_builds_per_app) { 3 } subject(:job) { PruneCompletedBuilds.new(max_retained_builds_per_app) } @@ -19,67 +19,67 @@ module Jobs::Runtime it 'deletes all the staged builds over the limit' do expect(BuildModel.count).to eq(0) - total = 50 - (1..50).each do |i| + total = 8 + (1..total).each do |i| BuildModel.make(id: i, state: BuildModel::STAGED_STATE, app: app, created_at: Time.now - total + i) end job.perform - expect(BuildModel.count).to eq(15) - expect(BuildModel.map(&:id)).to match_array((36..50).to_a) + expect(BuildModel.count).to eq(3) + expect(BuildModel.map(&:id)).to match_array((6..8).to_a) end it 'deletes all failed builds over the limit' do expect(BuildModel.count).to eq(0) - total = 50 - (1..50).each do |i| + total = 8 + (1..total).each do |i| BuildModel.make(id: i, state: BuildModel::FAILED_STATE, app: app, created_at: Time.now - total + i) end job.perform - expect(BuildModel.count).to eq(15) - expect(BuildModel.map(&:id)).to match_array((36..50).to_a) + expect(BuildModel.count).to eq(3) + expect(BuildModel.map(&:id)).to match_array((6..8).to_a) end it 'does NOT delete any staging builds over the limit' do expect(BuildModel.count).to eq(0) - total = 50 - (1..50).each do |i| + total = 8 + (1..total).each do |i| BuildModel.make(id: i, state: BuildModel::STAGING_STATE, app: app, created_at: Time.now - total + i) end job.perform - expect(BuildModel.count).to eq(50) - expect(BuildModel.map(&:id)).to match_array((1..50).to_a) + expect(BuildModel.count).to eq(8) + expect(BuildModel.map(&:id)).to match_array((1..8).to_a) end it 'does not delete in-flight builds over the limit' do - total = 60 - (1..20).each do |i| + total = 12 + (1..4).each do |i| BuildModel.make(id: i, state: BuildModel::STAGED_STATE, app: app, created_at: Time.now - total + i) end - (21..40).each do |i| + (5..8).each do |i| BuildModel.make(id: i, state: BuildModel::STAGING_STATE, app: app, created_at: Time.now - total + i) end - (41..60).each do |i| + (9..12).each do |i| BuildModel.make(id: i, state: BuildModel::STAGED_STATE, app: app, created_at: Time.now - total + i) end job.perform - expect(BuildModel.count).to be(35) - expect(BuildModel.order(Sequel.asc(:created_at), Sequel.asc(:id)).map(&:id)).to eq((21..40).to_a + (46..60).to_a) + expect(BuildModel.count).to be(7) + expect(BuildModel.order(Sequel.asc(:created_at), Sequel.asc(:id)).map(&:id)).to eq((5..8).to_a + (10..12).to_a) end it 'calls destroy on the BuildModel so association dependencies are respected' do expect(BuildModel.count).to eq(0) - 50.times do + 8.times do b = BuildModel.make(state: BuildModel::STAGED_STATE, app: app) BuildpackLifecycleDataModel.make(build: b) end @@ -97,7 +97,7 @@ module Jobs::Runtime expect(BuildModel.count).to eq(0) [app, app_the_second, app_the_third].each_with_index do |current_app, app_index| - total = 50 + total = 8 (1..total).each do |i| BuildModel.make(id: i + (1000 * app_index), state: BuildModel::STAGED_STATE, app: current_app, created_at: Time.now - total + i) end @@ -105,14 +105,14 @@ module Jobs::Runtime job.perform - expect(BuildModel.where(app:).count).to eq(15) - expect(BuildModel.where(app:).map(&:id)).to match_array((36..50).to_a) + expect(BuildModel.where(app:).count).to eq(3) + expect(BuildModel.where(app:).map(&:id)).to match_array((6..8).to_a) - expect(BuildModel.where(app: app_the_second).count).to eq(15) - expect(BuildModel.where(app: app_the_second).map(&:id)).to match_array((1036..1050).to_a) + expect(BuildModel.where(app: app_the_second).count).to eq(3) + expect(BuildModel.where(app: app_the_second).map(&:id)).to match_array((1006..1008).to_a) - expect(BuildModel.where(app: app_the_third).count).to eq(15) - expect(BuildModel.where(app: app_the_third).map(&:id)).to match_array((2036..2050).to_a) + expect(BuildModel.where(app: app_the_third).count).to eq(3) + expect(BuildModel.where(app: app_the_third).map(&:id)).to match_array((2006..2008).to_a) end end diff --git a/spec/unit/jobs/runtime/prune_completed_deployments_spec.rb b/spec/unit/jobs/runtime/prune_completed_deployments_spec.rb index f0a487971a5..e1f93004520 100644 --- a/spec/unit/jobs/runtime/prune_completed_deployments_spec.rb +++ b/spec/unit/jobs/runtime/prune_completed_deployments_spec.rb @@ -3,7 +3,7 @@ module VCAP::CloudController module Jobs::Runtime RSpec.describe PruneCompletedDeployments, job_context: :worker do - let(:max_retained_deployments_per_app) { 15 } + let(:max_retained_deployments_per_app) { 3 } subject(:job) { PruneCompletedDeployments.new(max_retained_deployments_per_app) } @@ -19,103 +19,103 @@ module Jobs::Runtime it 'deletes all the deployed deployments over the limit' do expect(DeploymentModel.count).to eq(0) - total = 50 - (1..50).each do |i| + total = 8 + (1..total).each do |i| DeploymentModel.make(id: i, state: DeploymentModel::DEPLOYED_STATE, app: app, created_at: Time.now - total + i) end job.perform - expect(DeploymentModel.count).to eq(15) - expect(DeploymentModel.map(&:id)).to match_array((36..50).to_a) + expect(DeploymentModel.count).to eq(3) + expect(DeploymentModel.map(&:id)).to match_array((6..8).to_a) end it 'deletes all canceled deployments over the limit' do expect(DeploymentModel.count).to eq(0) - total = 50 - (1..50).each do |i| + total = 8 + (1..total).each do |i| DeploymentModel.make(id: i, state: DeploymentModel::CANCELED_STATE, app: app, created_at: Time.now - total + i) end job.perform - expect(DeploymentModel.count).to eq(15) - expect(DeploymentModel.map(&:id)).to match_array((36..50).to_a) + expect(DeploymentModel.count).to eq(3) + expect(DeploymentModel.map(&:id)).to match_array((6..8).to_a) end it 'does NOT delete any deploying deployments over the limit' do expect(DeploymentModel.count).to eq(0) - total = 50 - (1..50).each do |i| + total = 8 + (1..total).each do |i| DeploymentModel.make(id: i, state: DeploymentModel::DEPLOYING_STATE, app: app, created_at: Time.now - total + i) end job.perform - expect(DeploymentModel.count).to eq(50) - expect(DeploymentModel.map(&:id)).to match_array((1..50).to_a) + expect(DeploymentModel.count).to eq(8) + expect(DeploymentModel.map(&:id)).to match_array((1..8).to_a) end it 'does NOT delete any prepaused deployments over the limit' do expect(DeploymentModel.count).to eq(0) - total = 50 - (1..50).each do |i| + total = 8 + (1..total).each do |i| DeploymentModel.make(id: i, state: DeploymentModel::PREPAUSED_STATE, app: app, created_at: Time.now - total + i) end job.perform - expect(DeploymentModel.count).to eq(50) - expect(DeploymentModel.map(&:id)).to match_array((1..50).to_a) + expect(DeploymentModel.count).to eq(8) + expect(DeploymentModel.map(&:id)).to match_array((1..8).to_a) end it 'does NOT delete any paused deployments over the limit' do expect(DeploymentModel.count).to eq(0) - total = 50 - (1..50).each do |i| + total = 8 + (1..total).each do |i| DeploymentModel.make(id: i, state: DeploymentModel::PAUSED_STATE, app: app, created_at: Time.now - total + i) end job.perform - expect(DeploymentModel.count).to eq(50) - expect(DeploymentModel.map(&:id)).to match_array((1..50).to_a) + expect(DeploymentModel.count).to eq(8) + expect(DeploymentModel.map(&:id)).to match_array((1..8).to_a) end it 'does NOT delete any canceling deployments over the limit' do expect(DeploymentModel.count).to eq(0) - total = 50 - (1..50).each do |i| + total = 8 + (1..total).each do |i| DeploymentModel.make(id: i, state: DeploymentModel::CANCELING_STATE, app: app, created_at: Time.now - total + i) end job.perform - expect(DeploymentModel.count).to eq(50) - expect(DeploymentModel.order(Sequel.asc(:created_at), Sequel.asc(:id)).map(&:id)).to eq((1..50).to_a) + expect(DeploymentModel.count).to eq(8) + expect(DeploymentModel.order(Sequel.asc(:created_at), Sequel.asc(:id)).map(&:id)).to eq((1..8).to_a) end it 'does not delete in-flight deployments over the limit' do - total = 60 - (1..20).each do |i| + total = 12 + (1..4).each do |i| DeploymentModel.make(id: i, state: DeploymentModel::DEPLOYED_STATE, app: app, created_at: Time.now - total + i) end - (21..40).each do |i| + (5..8).each do |i| DeploymentModel.make(id: i, state: DeploymentModel::DEPLOYING_STATE, app: app, created_at: Time.now - total + i) end - (41..60).each do |i| + (9..12).each do |i| DeploymentModel.make(id: i, state: DeploymentModel::DEPLOYED_STATE, app: app, created_at: Time.now - total + i) end job.perform - expect(DeploymentModel.count).to be(35) - expect(DeploymentModel.order(Sequel.asc(:created_at), Sequel.asc(:id)).map(&:id)).to eq((21..40).to_a + (46..60).to_a) + expect(DeploymentModel.count).to be(7) + expect(DeploymentModel.order(Sequel.asc(:created_at), Sequel.asc(:id)).map(&:id)).to eq((5..8).to_a + (10..12).to_a) end it 'destroys metadata associated with pruned deployments' do @@ -123,8 +123,8 @@ module Jobs::Runtime expect(DeploymentLabelModel.count).to eq(0) expect(DeploymentAnnotationModel.count).to eq(0) - total = 50 - (1..50).each do |i| + total = 8 + (1..total).each do |i| deployment = DeploymentModel.make(id: i, state: DeploymentModel::DEPLOYED_STATE, app: app, created_at: Time.now - total + i) DeploymentAnnotationModel.make(deployment: deployment, key_name: i, value: i) DeploymentLabelModel.make(deployment: deployment, key_name: i, value: i) @@ -132,18 +132,18 @@ module Jobs::Runtime job.perform - expect(DeploymentModel.count).to eq(15) - expect(DeploymentModel.map(&:id)).to match_array((36..50).to_a) - expect(DeploymentLabelModel.count).to eq(15) - expect(DeploymentLabelModel.map(&:value)).to match_array((36..50).map(&:to_s)) - expect(DeploymentAnnotationModel.count).to eq(15) - expect(DeploymentAnnotationModel.map(&:value)).to match_array((36..50).map(&:to_s)) + expect(DeploymentModel.count).to eq(3) + expect(DeploymentModel.map(&:id)).to match_array((6..8).to_a) + expect(DeploymentLabelModel.count).to eq(3) + expect(DeploymentLabelModel.map(&:value)).to match_array((6..8).map(&:to_s)) + expect(DeploymentAnnotationModel.count).to eq(3) + expect(DeploymentAnnotationModel.map(&:value)).to match_array((6..8).map(&:to_s)) end it 'destroys associated historical processes to maintain key constraints' do expect(DeploymentModel.count).to eq(0) - 50.times do + 8.times do d = DeploymentModel.make(state: DeploymentModel::DEPLOYED_STATE, app: app) DeploymentProcessModel.make(deployment: d) end @@ -161,7 +161,7 @@ module Jobs::Runtime expect(DeploymentModel.count).to eq(0) [app, app_the_second, app_the_third].each_with_index do |current_app, app_index| - total = 50 + total = 8 (1..total).each do |i| DeploymentModel.make(id: i + (1000 * app_index), state: DeploymentModel::DEPLOYED_STATE, app: current_app, created_at: Time.now - total + i) end @@ -169,14 +169,14 @@ module Jobs::Runtime job.perform - expect(DeploymentModel.where(app:).count).to eq(15) - expect(DeploymentModel.where(app:).map(&:id)).to match_array((36..50).to_a) + expect(DeploymentModel.where(app:).count).to eq(3) + expect(DeploymentModel.where(app:).map(&:id)).to match_array((6..8).to_a) - expect(DeploymentModel.where(app: app_the_second).count).to eq(15) - expect(DeploymentModel.where(app: app_the_second).map(&:id)).to match_array((1036..1050).to_a) + expect(DeploymentModel.where(app: app_the_second).count).to eq(3) + expect(DeploymentModel.where(app: app_the_second).map(&:id)).to match_array((1006..1008).to_a) - expect(DeploymentModel.where(app: app_the_third).count).to eq(15) - expect(DeploymentModel.where(app: app_the_third).map(&:id)).to match_array((2036..2050).to_a) + expect(DeploymentModel.where(app: app_the_third).count).to eq(3) + expect(DeploymentModel.where(app: app_the_third).map(&:id)).to match_array((2006..2008).to_a) end end diff --git a/spec/unit/jobs/runtime/prune_excess_app_revisions_spec.rb b/spec/unit/jobs/runtime/prune_excess_app_revisions_spec.rb index 5753f432829..26ee58fcb1f 100644 --- a/spec/unit/jobs/runtime/prune_excess_app_revisions_spec.rb +++ b/spec/unit/jobs/runtime/prune_excess_app_revisions_spec.rb @@ -3,7 +3,7 @@ module VCAP::CloudController module Jobs::Runtime RSpec.describe PruneExcessAppRevisions, job_context: :worker do - let(:max_retained_revisions_per_app) { 15 } + let(:max_retained_revisions_per_app) { 3 } subject(:job) { PruneExcessAppRevisions.new(max_retained_revisions_per_app) } @@ -19,15 +19,15 @@ module Jobs::Runtime it 'deletes all the revisions over the limit' do expect(RevisionModel.count).to eq(0) - total = 50 - (1..50).each do |i| + total = 8 + (1..total).each do |i| RevisionModel.make(version: i, app: app, created_at: Time.now - total + i) end job.perform expect(RevisionModel.count).to eq(max_retained_revisions_per_app) - expect(RevisionModel.map(&:version)).to match_array((36..50).to_a) + expect(RevisionModel.map(&:version)).to match_array((6..8).to_a) end context 'logging' do @@ -40,15 +40,15 @@ module Jobs::Runtime it 'logs the number of revisions deleted' do expect(RevisionModel.count).to eq(0) - total = 50 - (1..50).each do |i| + total = 8 + (1..total).each do |i| RevisionModel.make(version: i, app: app, created_at: Time.now - total + i) end job.perform expect(fake_logger).to have_received(:info).with('Cleaning up excess app revisions') - expect(fake_logger).to have_received(:info).with("Cleaned up 35 revision rows for app #{app.guid}") + expect(fake_logger).to have_received(:info).with("Cleaned up 5 revision rows for app #{app.guid}") end end @@ -57,8 +57,8 @@ module Jobs::Runtime expect(RevisionLabelModel.count).to eq(0) expect(RevisionAnnotationModel.count).to eq(0) - total = 50 - (1..50).each do |i| + total = 8 + (1..total).each do |i| revision = RevisionModel.make(version: i, app: app, created_at: Time.now - total + i) RevisionAnnotationModel.make(revision: revision, key_name: i, value: i) RevisionLabelModel.make(revision: revision, key_name: i, value: i) @@ -66,19 +66,19 @@ module Jobs::Runtime job.perform - expect(RevisionModel.count).to eq(15) - expect(RevisionModel.map(&:version)).to match_array((36..50).to_a) - expect(RevisionLabelModel.count).to eq(15) - expect(RevisionLabelModel.map(&:value)).to match_array((36..50).map(&:to_s)) - expect(RevisionAnnotationModel.count).to eq(15) - expect(RevisionAnnotationModel.map(&:value)).to match_array((36..50).map(&:to_s)) + expect(RevisionModel.count).to eq(3) + expect(RevisionModel.map(&:version)).to match_array((6..8).to_a) + expect(RevisionLabelModel.count).to eq(3) + expect(RevisionLabelModel.map(&:value)).to match_array((6..8).map(&:to_s)) + expect(RevisionAnnotationModel.count).to eq(3) + expect(RevisionAnnotationModel.map(&:value)).to match_array((6..8).map(&:to_s)) end it 'destroys associated process commands' do expect(RevisionModel.count).to eq(0) process_commands = [] - 50.times do |_i| + 8.times do |_i| revision = RevisionModel.make(app:) process_commands << revision.process_commands end @@ -86,10 +86,10 @@ module Jobs::Runtime expect do job.perform - end.to change(RevisionProcessCommandModel, :count).by(-35) + end.to change(RevisionProcessCommandModel, :count).by(-5) - expect(process_commands[0...35].none?(&:exists?)) - expect(process_commands[35...50].all?(&:exists?)) + expect(process_commands[0...5].none?(&:exists?)) + expect(process_commands[5...8].all?(&:exists?)) end context 'multiple apps' do @@ -100,7 +100,7 @@ module Jobs::Runtime expect(RevisionModel.count).to eq(0) [app, app_the_second, app_the_third].each_with_index do |current_app, app_index| - total = 50 + total = 8 (1..total).each do |i| RevisionModel.make(version: i + (1000 * app_index), app: current_app, created_at: Time.now - total + i) end @@ -108,14 +108,14 @@ module Jobs::Runtime job.perform - expect(RevisionModel.where(app:).count).to eq(15) - expect(RevisionModel.where(app:).map(&:version)).to match_array((36..50).to_a) + expect(RevisionModel.where(app:).count).to eq(3) + expect(RevisionModel.where(app:).map(&:version)).to match_array((6..8).to_a) - expect(RevisionModel.where(app: app_the_second).count).to eq(15) - expect(RevisionModel.where(app: app_the_second).map(&:version)).to match_array((1036..1050).to_a) + expect(RevisionModel.where(app: app_the_second).count).to eq(3) + expect(RevisionModel.where(app: app_the_second).map(&:version)).to match_array((1006..1008).to_a) - expect(RevisionModel.where(app: app_the_third).count).to eq(15) - expect(RevisionModel.where(app: app_the_third).map(&:version)).to match_array((2036..2050).to_a) + expect(RevisionModel.where(app: app_the_third).count).to eq(3) + expect(RevisionModel.where(app: app_the_third).map(&:version)).to match_array((2006..2008).to_a) end end diff --git a/spec/unit/lib/cloud_controller/errands/rotate_database_key_spec.rb b/spec/unit/lib/cloud_controller/errands/rotate_database_key_spec.rb index 392acd8e949..90e4caee043 100644 --- a/spec/unit/lib/cloud_controller/errands/rotate_database_key_spec.rb +++ b/spec/unit/lib/cloud_controller/errands/rotate_database_key_spec.rb @@ -199,8 +199,9 @@ def nilable_columns(entity) it 'does not change the updated_at field' do updated_at = app.reload.values[:updated_at] - sleep(1.5) # ensure that timestamp in `updated_at` would change - RotateDatabaseKey.perform(batch_size: 1) + Timecop.travel(Time.now + 2.seconds) do + RotateDatabaseKey.perform(batch_size: 1) + end expect(app.reload.values[:updated_at]).to eq(updated_at) end diff --git a/spec/unit/lib/uaa/uaa_token_decoder_spec.rb b/spec/unit/lib/uaa/uaa_token_decoder_spec.rb index af94c53dca5..7ef89cc6546 100644 --- a/spec/unit/lib/uaa/uaa_token_decoder_spec.rb +++ b/spec/unit/lib/uaa/uaa_token_decoder_spec.rb @@ -1,6 +1,13 @@ require 'spec_helper' require 'cloud_controller/uaa/uaa_token_decoder' +# Generating RSA-2048 keys is ~50-200ms on CI. The decoder spec used to do this +# in `let` blocks, regenerating a fresh key for every example in scope (~30+ +# examples). The behaviour under test does not depend on key randomness — only +# on which public key matches the signing private key — so we generate four +# distinct keys once at file load and reuse them. +RSA_TEST_KEYS = Array.new(4) { OpenSSL::PKey::RSA.new(2048) }.freeze + module VCAP::CloudController RSpec.describe UaaTokenDecoder do subject { UaaTokenDecoder.new(uaa_config) } @@ -202,7 +209,7 @@ module VCAP::CloudController allow(uaa_info).to receive_messages(validation_keys_hash: { 'key1' => { 'value' => rsa_key.public_key.to_pem } }) end - let(:rsa_key) { OpenSSL::PKey::RSA.new(2048) } + let(:rsa_key) { RSA_TEST_KEYS[0] } context 'when token is valid' do let(:token_content) do @@ -230,7 +237,7 @@ module VCAP::CloudController end describe 're-fetching key' do - let(:old_rsa_key) { OpenSSL::PKey::RSA.new(2048) } + let(:old_rsa_key) { RSA_TEST_KEYS[1] } it 'retries to decode token with newly fetched asymmetric key' do allow(uaa_info).to receive(:validation_keys_hash).and_return( @@ -376,7 +383,7 @@ module VCAP::CloudController end context 'when multiple asymmetric keys are used' do - let(:bad_rsa_key) { OpenSSL::PKey::RSA.new(2048) } + let(:bad_rsa_key) { RSA_TEST_KEYS[1] } let(:token_content) do { 'aud' => 'resource-id', @@ -410,7 +417,7 @@ module VCAP::CloudController end it 're-fetches keys when none of the keys are valid' do - other_bad_key = OpenSSL::PKey::RSA.new(2048) + other_bad_key = RSA_TEST_KEYS[2] allow(uaa_info).to receive(:validation_keys_hash).and_return( { 'bad_key' => { 'value' => bad_rsa_key.public_key.to_pem }, @@ -427,8 +434,8 @@ module VCAP::CloudController end it 'fails when re-fetched keys are also not valid' do - other_bad_key = OpenSSL::PKey::RSA.new(2048) - final_bad_key = OpenSSL::PKey::RSA.new(2048) + other_bad_key = RSA_TEST_KEYS[2] + final_bad_key = RSA_TEST_KEYS[3] allow(uaa_info).to receive(:validation_keys_hash).and_return( { 'bad_key' => { 'value' => bad_rsa_key.public_key.to_pem }, diff --git a/spec/unit/middleware/service_broker_rate_limiter_spec.rb b/spec/unit/middleware/service_broker_rate_limiter_spec.rb index c2f34b6c65a..256de3e16f5 100644 --- a/spec/unit/middleware/service_broker_rate_limiter_spec.rb +++ b/spec/unit/middleware/service_broker_rate_limiter_spec.rb @@ -20,10 +20,7 @@ module Middleware before do allow(ActionDispatch::Request).to receive(:new).and_return(fake_request) allow(logger).to receive(:info) - allow(app).to receive(:call) do - sleep(1) - [200, {}, 'a body'] - end + allow(app).to receive(:call).and_return([200, {}, 'a body']) end describe 'included requests' do @@ -84,7 +81,17 @@ module Middleware let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: endpoint, method: method) } it 'does not allow more than the max number of concurrent requests' do + gate = Queue.new + allow(app).to receive(:call) do + gate.pop + [200, {}, 'a body'] + end + threads = 2.times.map { Thread.new { middleware.call(env) } } + + Timeout.timeout(5) { Thread.pass until threads.any? { |t| !t.alive? } } + gate << :go + statuses = threads.map(&:join).map(&:value).map(&:first) expect(statuses).to include(200)