Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add patch for VCR for fixing VCR crashing in Ruby v3.1 #3216

Merged
merged 8 commits into from
Sep 19, 2023

Conversation

murny
Copy link
Contributor

@murny murny commented Sep 13, 2023

Context

Our test runner appears to be crashing after running:
image

The issue appears to be with Ruby v3.1 and VCR: vcr/vcr#907

This was fixed in v6.1 of VCR.

Simply bumping VCR up to v6.2 fixes the problem.

HOWEVER:

We purposely locked VCR to v5.0 though. We did this because of their license change from MIT to The Hippocratic License. I believe this is still relevant?

Discussion on this from Matt:
#1667

So if we feel this is still relevant, then it appears some people offered fixes for v5.0 in vcr/vcr#907 for us that we can maybe look into adopting as an alternative.

My gut feeling, is this is still relevant. So perhaps next steps I'll work on a quick fix using one of the suggestions in the PR and push this up instead. (Like Matt said, maybe the best solution is to go to webmock instead and just drop VCR, but this is obviously out of scope for now.)

UPDATE

Created an ticket in the backlog for migrating off of VCR: #3228

The approach I took was a simple patch that runs as an initializer, if in test environment, we load VCR and monkeypatch VCR to apply the fix. This seems to work well.

@pgwillia
Copy link
Member

Context

Our test runner appears to be crashing after running: image

The issue appears to be with Ruby v3.1 and VCR: vcr/vcr#907

This was fixed in v6.1 of VCR.

Simply bumping VCR up to v6.2 fixes the problem.

HOWEVER:

We purposely locked VCR to v5.0 though. We did this because of their license change from MIT to The Hippocratic License. I believe this is still relevant?

Discussion on this from Matt: #1667

So if we feel this is still relevant, then it appears some people offered fixes for v5.0 in vcr/vcr#907 for us that we can maybe look into adopting as an alternative.

My gut feeling, is this is still relevant. So perhaps next steps I'll work on a quick fix using one of the suggestions in the PR and push this up instead. (Like Matt said, maybe the best solution is to go to webmock instead and just drop VCR, but this is obviously out of scope for now.)

I share your gut feeling that this is still relevant and trust Matt's analysis of the situation and I suspect it hasn't changed.

Please proceed with the "quick fix" initializer from the suggestions and perhaps add a ticket to the backlog (I didn't see one on a quick search).

@murny
Copy link
Contributor Author

murny commented Sep 14, 2023

I share your gut feeling that this is still relevant and trust Matt's analysis of the situation and I suspect it hasn't changed.

Please proceed with the "quick fix" initializer from the suggestions and perhaps add a ticket to the backlog (I didn't see one on a quick search).

Sounds great 👍 . I'll do the quick fix and throw a new ticket in the backlog to maybe look into migrating off VCR to another gem (just use Webmock maybe might be the best course)

UPDATE: created #3228 and took the simple patch route instead

@murny murny changed the title Update VCR which has a fix for VCR crashing in Ruby v3.1 Add patch for VCR for fixing VCR crashing in Ruby v3.1 Sep 15, 2023
Copy link
Member

@pgwillia pgwillia left a comment

Choose a reason for hiding this comment

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

@murny murny merged commit abfcd59 into master Sep 19, 2023
2 checks passed
@murny murny deleted the fix-vcr-crash-on-running-tests branch September 19, 2023 16:23
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.

2 participants