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

Fix decoration spec #895

Merged
merged 5 commits into from
Jan 14, 2021
Merged

Conversation

tbuehlmann
Copy link
Contributor

@tbuehlmann tbuehlmann commented Jan 13, 2021

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
Copy link
Contributor Author

Looks like the integration specs are flaky, gonna check.

@tbuehlmann
Copy link
Contributor Author

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 tbuehlmann force-pushed the fix-a-failing-spec branch 3 times, most recently from 072da06 to f783d97 Compare January 13, 2021 18:43
@tbuehlmann
Copy link
Contributor Author

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.

Tobias Bühlmann added 4 commits January 14, 2021 12:00
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.
Newer versions of rails need a config/storage.yml in order
to work.
Ruby 3 no longer bundles the webrick gem so we need to
add it to the Gemfile.
When decorator_generator_spec.rb:51 was performed before or
after another spec (not sure), it would fail. It has to do
with requiring some rails constants.

To reproduce, run:

    bundle exec rspec spec/generators --seed=53929

Fixed by correctly requiring other files.
@tbuehlmann tbuehlmann force-pushed the fix-a-failing-spec branch 4 times, most recently from dc51c26 to 490f734 Compare January 14, 2021 13:06
Simplecov doesn't support Ruby 2.4 in newer versions
and the latest possible version generates reports
which are not processable by the code climate reporter.

So it seems we're stuck with simplecov 0.17.1 until
Ruby 2.4 is dropped:

codeclimate/test-reporter#413 (comment)
@tbuehlmann
Copy link
Contributor Author

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

@n-rodriguez
Copy link
Contributor

@codebycliff I think this one can be merged then #897

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

@n-rodriguez
Copy link
Contributor

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

@codebycliff
Copy link
Collaborator

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.

Copy link
Collaborator

@codebycliff codebycliff 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 for this! Sorry for my late reply on these.

@codebycliff codebycliff merged commit 4edc462 into drapergem:master Jan 14, 2021
@n-rodriguez
Copy link
Contributor

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.

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.

3 participants