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

Fixes issue with ActiveStorage::FixtureSet.blob causing a TypeError #2671

Merged
merged 1 commit into from May 12, 2023

Conversation

jaywhy
Copy link
Contributor

@jaywhy jaywhy commented Apr 19, 2023

This PR fixes an issue where calling ActiveStorage::FixtureSet.blob will error with TypeError: no implicit conversion of nil into String because the blob method expects ActiveStorage::FixtureSet.file_fixture_path to be set.

This PR adds a default value to ActiveStorage::FixtureSet.file_fixture_path, fixing the issue. It checks to see if it is defined because this is a new feature of Rails 7.

More information can be found in #2637.
fixes #2637

@pirj
Copy link
Member

pirj commented Apr 19, 2023

Nice! Would it be possible to add a scenario to features/file_fixture.feature?

@jaywhy
Copy link
Contributor Author

jaywhy commented Apr 20, 2023

@pirj I've added a scenario covering the issue.

@pirj
Copy link
Member

pirj commented May 1, 2023

Some feature failed. Can you please have a look?

@jaywhy
Copy link
Contributor Author

jaywhy commented May 1, 2023

@pirj thanks for the heads up.

The test fails because ActiveStorage::FixtureSet is not defined in Rails 6. I put an if check around the actual code but didn't consider the test.

I added a check around the test. This should fix the issue.

Copy link
Member

@pirj pirj left a comment

Choose a reason for hiding this comment

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

Thank you!

features/file_fixture.feature Outdated Show resolved Hide resolved
@pirj
Copy link
Member

pirj commented May 1, 2023

It seems that RubyGems times out, and causes red ci.

@pirj
Copy link
Member

pirj commented May 1, 2023

I don't understand why it won't let me force push.

@pirj
Copy link
Member

pirj commented May 1, 2023

From 106cca999d2aa1afff5c478631045c35cf0380df Mon Sep 17 00:00:00 2001
From: Jason Yates <jason@jasonyates.me>
Date: Wed, 19 Apr 2023 14:27:26 -0400
Subject: [PATCH] fixup!

---
 features/file_fixture.feature      |  7 +++----
 features/support/rails_versions.rb | 15 +++++++++++++++
 2 files changed, 18 insertions(+), 4 deletions(-)
 create mode 100644 features/support/rails_versions.rb

diff --git features/file_fixture.feature features/file_fixture.feature
index 845c280c..c6a765a5 100644
--- features/file_fixture.feature
+++ features/file_fixture.feature
@@ -37,6 +37,7 @@ Feature: Using `file_fixture`
     When I run `rspec spec/lib/file_spec.rb`
     Then the examples should all pass
 
+  @rails_post_7
   Scenario: Creating a ActiveStorage::Blob from a file fixture
     Given a file named "spec/fixtures/files/sample.txt" with:
       """
@@ -48,11 +49,9 @@ Feature: Using `file_fixture`
 
       RSpec.describe "blob" do
         it "creates a blob from a sample file" do
-          if ::Rails::VERSION::STRING >= '7'
-            expect(ActiveStorage::FixtureSet.blob(filename: "sample.txt")).to include("sample.txt")
-          end
+          expect(ActiveStorage::FixtureSet.blob(filename: "sample.txt")).to include("sample.txt")
         end
       end
       """
     When I run `rspec spec/lib/fixture_set_blob.rb`
-    Then the examples should all pass
\ No newline at end of file
+    Then the examples should all pass
diff --git features/support/rails_versions.rb features/support/rails_versions.rb
new file mode 100644
index 00000000..3b2e3ef7
--- /dev/null
+++ features/support/rails_versions.rb
@@ -0,0 +1,15 @@
+def rails_version
+  string_version = ENV.fetch("RAILS_VERSION", "~> 7.0.0")
+  if string_version == "main" || string_version.nil?
+    Float::INFINITY
+  else
+    string_version[/\d[\.-]\d/].tr('-', '.')
+  end
+end
+
+Before "@rails_post_7" do |scenario|
+  if rails_version.to_f < 7.0
+    warn "Skipping scenario #{scenario.name} on Rails v#{rails_version}"
+    skip_this_scenario
+  end
+end
-- 
2.39.1

@jaywhy
Copy link
Contributor Author

jaywhy commented May 1, 2023

@pirj I was able to add your patch. You're awesome 💯 👍

…veStorage::FixtureSet.file_fixture_path is not set.
@jaywhy
Copy link
Contributor Author

jaywhy commented May 5, 2023

I fixed the Rubocop issue. Hopefully that is everything 🤞.

@pirj pirj requested a review from JonRowe May 6, 2023 00:27
@JonRowe JonRowe merged commit 0a0564e into rspec:main May 12, 2023
15 checks passed
JonRowe added a commit that referenced this pull request May 12, 2023
JonRowe added a commit that referenced this pull request May 12, 2023
Fixes issue with ActiveStorage::FixtureSet.blob causing a TypeError
JonRowe added a commit that referenced this pull request May 12, 2023
@JonRowe
Copy link
Member

JonRowe commented May 31, 2023

Released in 6.0.3

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.

ActiveStorage::FixtureSet.blob incompatible with rspec
3 participants