Skip to content

Commit

Permalink
Fix RSpec mock expectations for Ruby 3 (#816)
Browse files Browse the repository at this point in the history
Something changed in the rspec-mocks gem between versions 3.10.2
and 3.10.3, because that's what causes tests to fail like this
on Ruby 3.1.0 and 3.0.3 locally and on the CI:

```
Failure/Error: monitor_transaction(name, env, &block)

  Appsignal received :monitor_transaction with unexpected arguments
    expected: ("perform_job.something", {:key=>:value})
         got: ("perform_job.something", {:key=>:value})
  Diff:
```

In this example the arguments are exactly the same, except they're not
the identical objects. This particular issue seems to be caused by a
recent change that for Ruby 3 keyword argument matching:
rspec/rspec-mocks#1460

## Rewriting specs

What helps is rewriting the tests to not mock method calls and check the
arguments given. Instead, assert what's being stored on the
transactions, like how we want all tests to be written as described in
issue #252.

## Adding curly brackets

For other specs that are a little more difficult to rewrite this way
right now, I've chosen to add additional curly brackets around the
argument expectations so that it's clear to Ruby/RSpec it's a hash
argument and not keywords arguments.

To solve this more permanently, we should rewrite the specs at some
point. We can remove these curly brackets if the issue is fixed in a
future rspec-mocks version as well.

[skip changeset]
  • Loading branch information
tombruijn authored Feb 7, 2022
1 parent 2c12f99 commit 0e9b72f
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,12 @@
end

it "should apply a strategy for each key" do
# TODO: additional curly brackets required for issue
# https://github.com/rspec/rspec-mocks/issues/1460
# rubocop:disable Style/BracesAroundHashParameters
expect(formatter).to receive(:apply_strategy)
.with(:sanitize_document, "_id" => 1)
.with(:sanitize_document, { "_id" => 1 })
# rubocop:enable Style/BracesAroundHashParameters

expect(formatter).to receive(:apply_strategy)
.with(:allow, "users")
Expand Down
6 changes: 5 additions & 1 deletion spec/lib/appsignal/integrations/mongo_ruby_driver_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,12 @@
end

it "should sanitize command" do
# TODO: additional curly brackets required for issue
# https://github.com/rspec/rspec-mocks/issues/1460
# rubocop:disable Style/BracesAroundHashParameters
expect(Appsignal::EventFormatter::MongoRubyDriver::QueryFormatter)
.to receive(:format).with("find", "foo" => "bar")
.to receive(:format).with("find", { "foo" => "bar" })
# rubocop:enable Style/BracesAroundHashParameters

subscriber.started(event)
end
Expand Down
14 changes: 11 additions & 3 deletions spec/lib/appsignal/integrations/sidekiq_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,14 @@ class DelayedTestClass; end
let(:error) { ExampleException }

it "creates a transaction and adds the error" do
# TODO: additional curly brackets required for issue
# https://github.com/rspec/rspec-mocks/issues/1460
# rubocop:disable Style/BracesAroundHashParameters
expect(Appsignal).to receive(:increment_counter)
.with("sidekiq_queue_job_count", 1, :queue => "default", :status => :failed)
.with("sidekiq_queue_job_count", 1, { :queue => "default", :status => :failed })
expect(Appsignal).to receive(:increment_counter)
.with("sidekiq_queue_job_count", 1, :queue => "default", :status => :processed)
.with("sidekiq_queue_job_count", 1, { :queue => "default", :status => :processed })
# rubocop:enable Style/BracesAroundHashParameters

expect do
perform_job { raise error, "uh oh" }
Expand Down Expand Up @@ -267,8 +271,12 @@ class DelayedTestClass; end

context "without an error" do
it "creates a transaction with events" do
# TODO: additional curly brackets required for issue
# https://github.com/rspec/rspec-mocks/issues/1460
# rubocop:disable Style/BracesAroundHashParameters
expect(Appsignal).to receive(:increment_counter)
.with("sidekiq_queue_job_count", 1, :queue => "default", :status => :processed)
.with("sidekiq_queue_job_count", 1, { :queue => "default", :status => :processed })
# rubocop:enable Style/BracesAroundHashParameters

perform_job

Expand Down
76 changes: 58 additions & 18 deletions spec/lib/appsignal_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -427,49 +427,89 @@
end

describe ".monitor_single_transaction" do
around { |example| keep_transactions { example.run } }

context "with a successful call" do
it "should call monitor_transaction and stop" do
expect(Appsignal).to receive(:monitor_transaction).with(
"perform_job.something",
:key => :value
).and_yield
it "calls monitor_transaction and Appsignal.stop" do
expect(Appsignal).to receive(:stop)

Appsignal.monitor_single_transaction("perform_job.something", :key => :value) do
Appsignal.monitor_single_transaction(
"perform_job.something",
:controller => :my_controller,
:action => :my_action
) do
# nothing
end

transaction = last_transaction
transaction_hash = transaction.to_h
expect(transaction_hash).to include(
"action" => "my_controller#my_action"
)
expect(transaction_hash["events"]).to match([
hash_including(
"name" => "perform_job.something",
"title" => "",
"body" => "",
"body_format" => Appsignal::EventFormatter::DEFAULT
)
])
end
end

context "with an erroring call" do
let(:error) { ExampleException.new }

it "should call monitor_transaction and stop and then raise the error" do
expect(Appsignal).to receive(:monitor_transaction).with(
"perform_job.something",
:key => :value
).and_yield
it "calls monitor_transaction and stop and re-raises the error" do
expect(Appsignal).to receive(:stop)

expect do
Appsignal.monitor_single_transaction("perform_job.something", :key => :value) do
Appsignal.monitor_single_transaction(
"perform_job.something",
:controller => :my_controller,
:action => :my_action
) do
raise error
end
end.to raise_error(error)

transaction = last_transaction
transaction_hash = transaction.to_h
expect(transaction_hash).to include(
"action" => "my_controller#my_action"
)
expect(transaction_hash["events"]).to match([
hash_including(
"name" => "perform_job.something",
"title" => "",
"body" => "",
"body_format" => Appsignal::EventFormatter::DEFAULT
)
])
end
end
end

describe ".tag_request" do
before { allow(Appsignal::Transaction).to receive(:current).and_return(transaction) }
let(:transaction) { http_request_transaction }
around do |example|
start_agent
with_current_transaction transaction do
keep_transactions { example.run }
end
end

context "with transaction" do
let(:transaction) { double }
it "should call set_tags on transaction" do
expect(transaction).to receive(:set_tags).with("a" => "b")
end
it "calls set_tags on the current transaction" do
Appsignal.tag_request("a" => "b")
transaction.complete # Manually trigger transaction sampling

after { Appsignal.tag_request("a" => "b") }
expect(transaction.to_h).to include(
"sample_data" => hash_including(
"tags" => { "a" => "b" }
)
)
end
end

context "without transaction" do
Expand Down
10 changes: 10 additions & 0 deletions spec/support/helpers/transaction_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,16 @@ def clear_current_transaction!
Thread.current[:appsignal_transaction] = nil
end

# Set the current for the duration of the given block.
#
# Helper for {set_current_transaction} and {clear_current_transaction!}
def with_current_transaction(transaction)
set_current_transaction transaction
yield
ensure
clear_current_transaction!
end

# Track the AppSignal transaction JSON when a transaction gets completed
# ({Appsignal::Transaction.complete}).
#
Expand Down

0 comments on commit 0e9b72f

Please sign in to comment.