Skip to content

Conversation

@tdutreui-solocal
Copy link

@tdutreui-solocal tdutreui-solocal commented Sep 5, 2024

lib/gcr.rb Outdated
# Whether cassettes should be compressed to zz
#
# Returns a boolean
def compress

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def compress
def compress?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied, thx

Copy link

@danielmbarlow danielmbarlow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks TD. Just had a smal comment to make about a minor detail

Comment on lines 175 to 179
raise GCR::NoRecording.new(["Unrecorded request :",
"called #{calls_count} #{(calls_count > 1) ? "times" : "time"}, (recorded #{interactions.size})",
req.class_name,
req.body]
.join("\n"))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a little difficult to read maybe we could extract some variables and improve the whitespace

@tdutreui-solocal
Copy link
Author

@asux Hi !
Do you accept Pull requests?

I propose you these improvements that we have been testing for some months now, without major issues.
We are using Rspec + VCR + GCR to make integration tests with the Google Ads API (using the official client)
The only downside is that sometimes Rspec hangs at the end of the execution, so we had to add this at_exit hook to circumvent it, in a shared context.

RSpec.shared_context "using GCR and VCR" do
  around(:example) do |ex|
    cassette_name = ex.metadata[:full_description].parameterize.underscore
    VCR.use_cassette(cassette_name) do
      GCR.with_cassette cassette_name do
        ex.run
      end
    end
  end

  at_exit do
    GCR.stub.instance_variable_get(:@ch).close
    GC.start # garbage collects stuff might fix Rspec hangs, see https://github.com/grpc/grpc/issues/38442
  end
end

@asux
Copy link

asux commented Aug 25, 2025

Hi @tdutreui-solocal. I'm bit loose context about this GCR, since not use it in recent project. Generally, your code makes sense to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants