Skip to content

Commit 91b5b32

Browse files
ivoanjoivoanjo
committed
Simplify testing for rollbar warning
The approach taken in my first attempt to test this change was rather heavy handed -- needing a lot of changes to the appraisals/rakefile and multiple runs of the test to validate. As suggested during review, let's instead mock out the Gem apis, which are documented and pretty stable, and avoid the impact on the test suite running time/complexity.
1 parent 662a44e commit 91b5b32

File tree

4 files changed

+15
-145
lines changed

4 files changed

+15
-145
lines changed

‎Appraisals

Lines changed: 0 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -72,15 +72,6 @@ elsif Gem::Version.new('2.0.0') <= Gem::Version.new(RUBY_VERSION) \
7272
gem 'lograge', '< 0.4'
7373
end
7474

75-
appraise 'rollbar-incompatible' do
76-
gem 'rollbar', '= 3.1.1'
77-
end
78-
79-
appraise 'rollbar-compatible' do
80-
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
81-
#gem 'rollbar', '= 3.1.2'
82-
end
83-
8475
appraise 'contrib-old' do
8576
gem 'active_model_serializers', '~> 0.9.0'
8677
gem 'activerecord', '3.2.22.5'
@@ -203,15 +194,6 @@ elsif Gem::Version.new('2.1.0') <= Gem::Version.new(RUBY_VERSION) \
203194
gem 'lograge'
204195
end
205196

206-
appraise 'rollbar-incompatible' do
207-
gem 'rollbar', '= 3.1.1'
208-
end
209-
210-
appraise 'rollbar-compatible' do
211-
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
212-
#gem 'rollbar', '= 3.1.2'
213-
end
214-
215197
appraise 'contrib-old' do
216198
gem 'active_model_serializers', '~> 0.9.0'
217199
gem 'activerecord', '3.2.22.5'
@@ -386,15 +368,6 @@ elsif Gem::Version.new('2.2.0') <= Gem::Version.new(RUBY_VERSION) \
386368
gem 'lograge'
387369
end
388370

389-
appraise 'rollbar-incompatible' do
390-
gem 'rollbar', '= 3.1.1'
391-
end
392-
393-
appraise 'rollbar-compatible' do
394-
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
395-
#gem 'rollbar', '= 3.1.2'
396-
end
397-
398371
appraise 'contrib' do
399372
gem 'actionpack'
400373
gem 'actionview'
@@ -588,15 +561,6 @@ elsif Gem::Version.new('2.3.0') <= Gem::Version.new(RUBY_VERSION) \
588561

589562
(3..4).each { |v| gem_cucumber(v) }
590563

591-
appraise 'rollbar-incompatible' do
592-
gem 'rollbar', '= 3.1.1'
593-
end
594-
595-
appraise 'rollbar-compatible' do
596-
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
597-
#gem 'rollbar', '= 3.1.2'
598-
end
599-
600564
appraise 'contrib' do
601565
gem 'actionpack'
602566
gem 'actionview'
@@ -700,15 +664,6 @@ elsif Gem::Version.new('2.4.0') <= Gem::Version.new(RUBY_VERSION) \
700664

701665
(3..4).each { |v| gem_cucumber(v) }
702666

703-
appraise 'rollbar-incompatible' do
704-
gem 'rollbar', '= 3.1.1'
705-
end
706-
707-
appraise 'rollbar-compatible' do
708-
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
709-
#gem 'rollbar', '= 3.1.2'
710-
end
711-
712667
appraise 'contrib' do
713668
gem 'actionpack'
714669
gem 'actionview'
@@ -861,15 +816,6 @@ elsif Gem::Version.new('2.5.0') <= Gem::Version.new(RUBY_VERSION) \
861816

862817
(3..5).each { |v| gem_cucumber(v) }
863818

864-
appraise 'rollbar-incompatible' do
865-
gem 'rollbar', '= 3.1.1'
866-
end
867-
868-
appraise 'rollbar-compatible' do
869-
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
870-
#gem 'rollbar', '= 3.1.2'
871-
end
872-
873819
appraise 'contrib' do
874820
gem 'actionpack'
875821
gem 'actionview'
@@ -1013,15 +959,6 @@ elsif Gem::Version.new('2.6.0') <= Gem::Version.new(RUBY_VERSION) \
1013959

1014960
(3..5).each { |v| gem_cucumber(v) }
1015961

1016-
appraise 'rollbar-incompatible' do
1017-
gem 'rollbar', '= 3.1.1'
1018-
end
1019-
1020-
appraise 'rollbar-compatible' do
1021-
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
1022-
#gem 'rollbar', '= 3.1.2'
1023-
end
1024-
1025962
appraise 'contrib' do
1026963
gem 'actionpack'
1027964
gem 'actionview'
@@ -1167,15 +1104,6 @@ elsif Gem::Version.new('2.7.0') <= Gem::Version.new(RUBY_VERSION)
11671104

11681105
(3..5).each { |v| gem_cucumber(v) }
11691106

1170-
appraise 'rollbar-incompatible' do
1171-
gem 'rollbar', '= 3.1.1'
1172-
end
1173-
1174-
appraise 'rollbar-compatible' do
1175-
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
1176-
#gem 'rollbar', '= 3.1.2'
1177-
end
1178-
11791107
appraise 'contrib' do
11801108
gem 'actionpack'
11811109
gem 'actionview'

‎Rakefile

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,6 @@ namespace :spec do
5757
t.rspec_opts = args.to_a.join(' ')
5858
end
5959

60-
RSpec::Core::RakeTask.new(:rollbar) do |t, args|
61-
t.pattern = 'spec/ddtrace/profiling/tasks/setup_spec.rb'
62-
t.rspec_opts = args.to_a.join(' ')
63-
end
64-
6560
RSpec::Core::RakeTask.new(:contrib) do |t, args|
6661
contrib_paths = [
6762
'analytics',
@@ -264,10 +259,6 @@ task :ci do
264259
declare 'bundle exec appraisal rails32-postgres rake spec:action_view'
265260
declare 'bundle exec appraisal rails32-mysql2 rake spec:active_record'
266261
declare 'bundle exec appraisal rails32-postgres rake spec:active_support'
267-
268-
declare 'bundle exec appraisal rollbar-incompatible rake spec:rollbar'
269-
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
270-
# declare 'bundle exec appraisal rollbar-compatible rake spec:rollbar'
271262
end
272263
elsif Gem::Version.new('2.1.0') <= Gem::Version.new(RUBY_VERSION) \
273264
&& Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.2.0')
@@ -331,10 +322,6 @@ task :ci do
331322
declare 'bundle exec appraisal rails32-postgres rake spec:action_view'
332323
declare 'bundle exec appraisal rails32-mysql2 rake spec:active_record'
333324
declare 'bundle exec appraisal rails32-postgres rake spec:active_support'
334-
335-
declare 'bundle exec appraisal rollbar-incompatible rake spec:rollbar'
336-
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
337-
# declare 'bundle exec appraisal rollbar-compatible rake spec:rollbar'
338325
end
339326
elsif Gem::Version.new('2.2.0') <= Gem::Version.new(RUBY_VERSION)\
340327
&& Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.3.0')
@@ -411,10 +398,6 @@ task :ci do
411398
declare 'bundle exec appraisal rails4-postgres rake spec:rails'
412399
declare 'bundle exec appraisal rails5-mysql2 rake spec:rails'
413400
declare 'bundle exec appraisal rails5-postgres rake spec:rails'
414-
415-
declare 'bundle exec appraisal rollbar-incompatible rake spec:rollbar'
416-
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
417-
# declare 'bundle exec appraisal rollbar-compatible rake spec:rollbar'
418401
end
419402
elsif Gem::Version.new('2.3.0') <= Gem::Version.new(RUBY_VERSION) \
420403
&& Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.4.0')
@@ -496,10 +479,6 @@ task :ci do
496479
# explicitly test resque-2x compatability
497480
declare 'bundle exec appraisal resque2-redis3 rake spec:resque'
498481
declare 'bundle exec appraisal resque2-redis4 rake spec:resque'
499-
500-
declare 'bundle exec appraisal rollbar-incompatible rake spec:rollbar'
501-
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
502-
# declare 'bundle exec appraisal rollbar-compatible rake spec:rollbar'
503482
end
504483
elsif Gem::Version.new('2.4.0') <= Gem::Version.new(RUBY_VERSION) \
505484
&& Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.5.0')
@@ -571,10 +550,6 @@ task :ci do
571550
# explicitly test cucumber compatibility
572551
declare 'bundle exec appraisal cucumber3 rake spec:cucumber'
573552
declare 'bundle exec appraisal cucumber4 rake spec:cucumber'
574-
575-
declare 'bundle exec appraisal rollbar-incompatible rake spec:rollbar'
576-
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
577-
# declare 'bundle exec appraisal rollbar-compatible rake spec:rollbar'
578553
end
579554
elsif Gem::Version.new('2.5.0') <= Gem::Version.new(RUBY_VERSION) \
580555
&& Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.6.0')
@@ -657,10 +632,6 @@ task :ci do
657632
declare 'bundle exec appraisal cucumber3 rake spec:cucumber'
658633
declare 'bundle exec appraisal cucumber4 rake spec:cucumber'
659634
declare 'bundle exec appraisal cucumber5 rake spec:cucumber'
660-
661-
declare 'bundle exec appraisal rollbar-incompatible rake spec:rollbar'
662-
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
663-
# declare 'bundle exec appraisal rollbar-compatible rake spec:rollbar'
664635
elsif Gem::Version.new('2.6.0') <= Gem::Version.new(RUBY_VERSION) \
665636
&& Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.7.0')
666637
# Main library
@@ -745,10 +716,6 @@ task :ci do
745716
declare 'bundle exec appraisal cucumber3 rake spec:cucumber'
746717
declare 'bundle exec appraisal cucumber4 rake spec:cucumber'
747718
declare 'bundle exec appraisal cucumber5 rake spec:cucumber'
748-
749-
declare 'bundle exec appraisal rollbar-incompatible rake spec:rollbar'
750-
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
751-
# declare 'bundle exec appraisal rollbar-compatible rake spec:rollbar'
752719
end
753720
elsif Gem::Version.new('2.7.0') <= Gem::Version.new(RUBY_VERSION)
754721
# Main library
@@ -832,10 +799,6 @@ task :ci do
832799
declare 'bundle exec appraisal cucumber3 rake spec:cucumber'
833800
declare 'bundle exec appraisal cucumber4 rake spec:cucumber'
834801
declare 'bundle exec appraisal cucumber5 rake spec:cucumber'
835-
836-
declare 'bundle exec appraisal rollbar-incompatible rake spec:rollbar'
837-
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
838-
# declare 'bundle exec appraisal rollbar-compatible rake spec:rollbar'
839802
end
840803
end
841804
end

‎lib/ddtrace/profiling/tasks/setup.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ def check_warnings!
8484
warn_if_incompatible_rollbar_gem_detected
8585
end
8686

87+
# See https://github.com/rollbar/rollbar-gem/pull/1018 for details on the incompatibility
8788
def warn_if_incompatible_rollbar_gem_detected
8889
incompatible_rollbar_versions = Gem::Requirement.new('<= 3.1.1')
8990

‎spec/ddtrace/profiling/tasks/setup_spec.rb

Lines changed: 14 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -361,26 +361,19 @@
361361
describe '#warn_if_incompatible_rollbar_gem_detected' do
362362
subject(:warn_if_incompatible_rollbar_gem_detected) { task.warn_if_incompatible_rollbar_gem_detected }
363363

364-
# Testing this code is slightly awkward for two reasons:
365-
# 1. We want to avoid using exactly the same code (gem apis) in the tests than we have for production.
366-
# 2. A given Ruby installation can only be in one of the possible states (no rollbar installed, old rollbar
367-
# installed, etc)
368-
#
369-
# To address this, we:
370-
# 1. Shell out to gem to detect if rollbar is installed or not (rather than using the same gem apis we want to test)
371-
# 2. Rely on the Appraisal gem test setup to be able to check the multiple cases in different runs
372-
373-
before(:context) do
374-
@rollbar_versions_installed =
375-
`gem list`.each_line.select { |it| it.start_with?('rollbar ') }.sort.map(&:strip)
364+
let(:last_version_of_rollbar_affected) { '3.1.1' }
365+
366+
before do
367+
# Simulate the result of the gem apis, so that we can check different combinations of having or not having the
368+
# rollbar gem and affected versions
369+
expect(Gem::Specification)
370+
.to receive(:find_all_by_name)
371+
.with('rollbar', Gem::Requirement.new("<= #{last_version_of_rollbar_affected}"))
372+
.and_return(rollbar_versions_found)
376373
end
377374

378375
context 'when rollbar gem is not installed' do
379-
before do
380-
if @rollbar_versions_installed.any?
381-
skip "Current gem environment (#{@rollbar_versions_installed}) not setup for this test"
382-
end
383-
end
376+
let(:rollbar_versions_found) { [] }
384377

385378
it 'does not display a warning to STDOUT' do
386379
expect(STDOUT).to_not receive(:puts)
@@ -390,19 +383,9 @@
390383
end
391384

392385
context 'when compatible version of rollbar gem is installed' do
393-
before do
394-
skip %q(FIXME NEW ROLLBAR NEEDED: This test cannot be enabled until a new version of rollbar (> 3.1.1) including
395-
"https://github.com/rollbar/rollbar-gem/pull/1018" is released.
396-
397-
Once this new version is released, we need to:
398-
1. Remove this skip
399-
2. Update the `Appraisals` file to enable the new version in the 'compatible-rollbar' appraisals~
400-
3. Enable the 'compatible-rollbar' validations in the `Rakefile`.)
401-
402-
if @rollbar_versions_installed != ['rollbar (3.1.2)']
403-
skip "Current gem environment (#{@rollbar_versions_installed}) not setup for this test"
404-
end
405-
end
386+
# same as "no gem installed" because we use a version requirement when
387+
# calling find_all_by_name, so only incompatible versions get returned
388+
let(:rollbar_versions_found) { [] }
406389

407390
it 'does not display a warning to STDOUT' do
408391
expect(STDOUT).to_not receive(:puts)
@@ -412,15 +395,10 @@
412395
end
413396

414397
context 'when incompatible version of rollbar gem is installed' do
415-
before do
416-
if @rollbar_versions_installed != ['rollbar (3.1.1)']
417-
skip "Current gem environment (#{@rollbar_versions_installed}) not setup for this test"
418-
end
419-
end
398+
let(:rollbar_versions_found) { [instance_double(Gem::Specification), instance_double(Gem::Specification)] }
420399

421400
it 'displays a warning to STDOUT' do
422401
expect(STDOUT).to receive(:puts) do |message|
423-
STDERR.puts(message)
424402
expect(message).to include('Incompatible version of the rollbar')
425403
end
426404

0 commit comments

Comments
 (0)