drapergem / draper

Decorators/View-Models for Rails Applications
MIT License
5.23k stars 527 forks source link

Fix decoration spec #895

Closed tbuehlmann closed 3 years ago

tbuehlmann commented 3 years ago

Draper::CollectionDecorator#{is_a?,kind_of?} triggers decorating the object. When stubbing a method call on a CollectionDecorator, RSpec itself will call #is_a? on the object. See:

https://github.com/rspec/rspec-mocks/blob/ea4d8ea4532480500462cc7a633d3408656a4dcd/lib/rspec/mocks/proxy.rb#L38

That's why the object will always be decorated when stubbing a method on it.

We can circumvent this issue by directly checking the definition of @decorated_collection.

Edit:

This PR got bigger than intended, but it fixes more failing and flaky specs.

tbuehlmann commented 3 years ago

Looks like the integration specs are flaky, gonna check.

tbuehlmann commented 3 years ago

Right, so they weren't flaky. Ruby 3 doesn't bundle the webrick gem anymore, so starting a server without mentioning it in the Gemfile wouldn't work. Also, newer versions of Rails need an activestorage configuration in order to work. Added those.

tbuehlmann commented 3 years ago

Righty, but I found a real flaky spec now. Could reproduce with bundle exec rspec spec/generators --seed=53929. Fixed now and hopefully CI will pass now.

tbuehlmann commented 3 years ago

@mgerst: Finally fixed it to green, but had to fixate simplecov to an old version.

n-rodriguez commented 3 years ago

@codebycliff I think this one can be merged then https://github.com/drapergem/draper/pull/897

Then we'll have a green CI ready to be tested against Ruby 3.0 \o/

n-rodriguez commented 3 years ago

Also it could be great to have appraisal to test against different versions of Rails. wdyt?

codebycliff commented 3 years ago

I think that would be awesome if someone wants to whip it up. We should also test against Ruby 3, but didn't want to make that change as part of the CI change.

n-rodriguez commented 3 years ago

I think that would be awesome if someone wants to whip it up. We should also test against Ruby 3, but didn't want to make that change as part of the CI change.

I can do a PR for that.