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.

Sign up for GitHub

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

Jump to bottom

Check if TestCase responds to the fixture_path setter #2349

Closed
wants to merge 2 commits into from
Closed

Check if TestCase responds to the fixture_path setter #2349

wants to merge 2 commits into from

Conversation

koenpunt
Copy link
Contributor

The existence of the getter doesn't mean there's also a setter, so this resulted in failures like the following:

1) Admin::CollectionsController POST #create creates a record
     Failure/Error: banner_image: fixture_file_upload('alpaca.jpg', 'image/jpg')

     NoMethodError:
       undefined method `fixture_path=' for ActionController::TestCase:Class
       Did you mean?  fixture_path

@benoittgt
Copy link
Member

Do you think it is possible to add a test?

@koenpunt
Copy link
Contributor Author

I think so, will have a look tonight.

@JonRowe
Copy link
Member

JonRowe commented May 27, 2020

Are you seeing an issue from this @koenpunt? I think this method is always available isn't it?

@koenpunt
Copy link
Contributor Author

Yeah I am, fixture_file_upload method doesn't work in my Rails 6 application

@JonRowe
Copy link
Member

JonRowe commented May 27, 2020

In that case we'd definitely like an integration spec for this so we can catch such problems in future 😂

@benoittgt
Copy link
Member

@koenpunt, if you need any help feel free to ask. I will be happy to help you.

@benoittgt
Copy link
Member

Little bump on this one. Still available if you need help @koenpunt.

@koenpunt
Copy link
Contributor Author

koenpunt commented Jul 2, 2020

Yes, sorry, just started a new job so didn't really had time to take a look at this again. Will try to get back to this ASAP.

@koenpunt
Copy link
Contributor Author

koenpunt commented Jul 14, 2020 •
edited

I've now added a test which I hoped was going to fail without my changes, but that isn't the case. Any pointers for what I can try to implement the test differently?

@koenpunt
Copy link
Contributor Author

After some further investigation I realised that I actually added the fixture_path method to ActionDispatch::TestProcess to allow usage of fixture_file_upload from FactoryBot factories. But I didn't implement the setter, so this caused rspec-rails to raise the NoMethodError.

I've now changed my implementation to not modify ActionDispatch::TestProcess, but instead monkey patch the fixture_file_upload method into FactoryBot, which solves the issue.

That said, I still think the respond_to?(:fixture_path) isn't correct, because it checks the getter, but then uses the setter.

Should I create a test where I add the fixture_path method, to demonstrate the failure?

@JonRowe JonRowe mentioned this pull request Jul 27, 2020
Merged
@JonRowe JonRowe changed the base branch from master to main August 2, 2020 02:13
@JonRowe JonRowe mentioned this pull request Aug 24, 2020
Merged
sudara added a commit to sudara/rspec-rails that referenced this pull request Sep 20, 2020
@sudara
ActionDispatch::TestProcess needs to access :file_fixture_path on Rai…
9e7de8c 
…ls 6.1

I had a similar problem to @koenpunt in rspec#2349 where `fixture_file_upload` stopped working.

```
     NoMethodError:
       undefined method `file_fixture_path' for RSpec::Rails::FixtureFileUploadSupport::RailsFixtureFileWrapper:Class
       Did you mean?  fixture_path
     # /actionpack/lib/action_dispatch/testing/test_process.rb:25:in `fixture_file_upload'
     # /rspec-rails-dd093928f021/lib/rspec/rails/fixture_file_upload_support.rb:5:in `fixture_file_upload'

```

In my case, it's because I'm on Rails 6.1 alpha. As of [these recent changes]( rails/rails#39086), `ActionController::TestCase`  now needs access to `fixture_file_path` within `fixture_file_upload`.

There's not currently any CI for Rails 6.1 (as it's not out yet) but I thought I would get a head start on things.
@sudara sudara mentioned this pull request Sep 20, 2020
Closed
@JonRowe
Copy link
Member

JonRowe commented Sep 23, 2020

Closed by #2370

@JonRowe JonRowe closed this Sep 23, 2020
@koenpunt koenpunt deleted the patch-1 branch June 3, 2021 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benoittgt benoittgt benoittgt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@koenpunt @benoittgt @JonRowe