Skip to content

Conversation

xaviershay
Copy link
Member

This was a terrible example and should never have been here. This
replacement isn't great, but at least is not obviously bad.

Unfortunately we cannot provide default implementations of these because
we don't have sufficient information about the data model. Specs that
require this are marked pending.

I gave up on the default approach: it doesn't work at all for models
with no attributes, and is a crapshoot if they do.

@cupakromer @JonRowe @myronmarston

Fixes #1008

This was a terrible example and should never have been here. This
replacement isn't great, but at least is not obviously bad.

Unfortunately we cannot provide default implementations of these because
we don't have sufficient information about the data model. Specs that
require this are marked pending.

I gave up on the default approach: it doesn't work at all for models
with no attributes, and is a crapshoot if they do.
@JonRowe
Copy link
Member

JonRowe commented May 27, 2014

This gets my 👍 I'd much rather we make people think about their specs than try to blindly use scaffolded code, I'd actually be ok with them failing rather than being skipped when they're not setup, but I think some people will argue that's not very friendly to beginners.

@cupakromer
Copy link
Member

I don't have time right now, but I plan on reviewing this tomorrow AM; US EST time.

@myronmarston
Copy link
Member

This LGTM, FWIW.

@cupakromer
Copy link
Member

Overall, I think it looks pretty good. I want to sit on it just for a bit (no longer than today) to think it over. I would prefer to use pending instead of skip for the attributes. However, this provides some challenges based on how those let variables are used.

Several specs simply use them to create! initial objects. Which causes them to fail, even though they could pass with an empty attribute hash. However, providing an empty attribute hash causes Rails strong params to complain that the params are empty. :-/

@cupakromer cupakromer added this to the 3.0.0 milestone May 27, 2014
@xaviershay
Copy link
Member Author

I used skip because pending would cause a failure in the ones that don't have any assertions yet.

create! should be provided with valid_attributes, the empty hash issue you describe is part of the reason I removed example_valid_attributes.

@cupakromer
Copy link
Member

I don't really have a good alternative at the moment. We can always adjust it in dot release since it's only a generator.

ship it :shipit:

xaviershay added a commit that referenced this pull request May 28, 2014
Replace mocking in generated controller specs with integration tests.
@xaviershay xaviershay merged commit 6ca759a into master May 28, 2014
@xaviershay xaviershay deleted the issue-1008 branch May 28, 2014 03:05
amatsuda added a commit to asakusarb/action_args that referenced this pull request Aug 6, 2015
because the controller spec generator is no more a thing in rspec-rails 3 since rspec/rspec-rails#1046

thank you @masarakki for pointing this out 🤘
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider removing allow_any_instance_of from the controller spec scaffold templates
4 participants