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

Use verified doubles in specs #260

Merged
merged 2 commits into from
May 23, 2015
Merged

Conversation

MGotink
Copy link

@MGotink MGotink commented May 17, 2015

As noted by @bf4 in #258.

@MGotink MGotink force-pushed the verified_doubles branch from c54a854 to 0dbe22b Compare May 17, 2015 20:27
]
@lines = double(smells: @smells)
@lines = instance_double(Reek::Core::Examiner, smells: @smells)
Copy link
Member

Choose a reason for hiding this comment

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

so, this test will fail on Reek versions before the Reek::Examiner was renamed to Reek::Core::Examiner, but I'm ok with it for now.

@MGotink MGotink force-pushed the verified_doubles branch from 0dbe22b to 3bbaeb7 Compare May 17, 2015 20:34
@bf4
Copy link
Member

bf4 commented May 17, 2015

Is missing config in spec_helper

config.mock_with :rspec do |mocks|
    mocks.syntax = :expect
+    mocks.verify_partial_doubles = true
 end

@MGotink MGotink mentioned this pull request May 17, 2015
@MGotink
Copy link
Author

MGotink commented May 17, 2015

Hmm, 1.9.2 is failing, no idea why, do we still want to support it?

@bf4
Copy link
Member

bf4 commented May 17, 2015

It's probably evaling the const before it's defined by the require statement... so, either we can refactor now to use the code from the generator, or just do the release without this then take our time on the refactor

@bf4
Copy link
Member

bf4 commented May 17, 2015

Since you're doing the work right now, you can decide :)

@MGotink MGotink force-pushed the verified_doubles branch 3 times, most recently from 395dfc7 to 5ca92f3 Compare May 18, 2015 19:23
@MGotink MGotink force-pushed the verified_doubles branch from 5ca92f3 to 8a91017 Compare May 18, 2015 19:52
@@ -43,51 +43,63 @@
MetricFu::Configuration.run {}
allow(File).to receive(:directory?).and_return(true)
@reek = MetricFu::ReekGenerator.new
@examiner = @reek.send(:examiner)
@smell_warning = Reek.const_defined?(:SmellWarning) ? Reek.const_get(:SmellWarning) : Reek.const_get(:Smells).const_get(:SmellWarning)
if @smell_warning.instance_methods.include?(:subclass)
Copy link
Author

Choose a reason for hiding this comment

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

I'm not too happy with this kind of hacks in the specs, but I can't think of a better solution right now.
Ruby 1.9.2. is using an older reek version which doesn't implement SmellWarning#smell_type, due to which the verified double fails.
OTOH the older reek output is tested separately.

@bf4, maybe you have a better solution?

@MGotink
Copy link
Author

MGotink commented May 22, 2015

@bf4, any ideas for the old reek fix, or just keep it this way for now?

@bf4
Copy link
Member

bf4 commented May 22, 2015

I suppose this way for now...

B mobile phone

On May 22, 2015, at 2:37 PM, Martin Gotink [email protected] wrote:

@bf4, any ideas for the old reek fix, or just keep it this way for now?


Reply to this email directly or view it on GitHub.

MGotink added a commit that referenced this pull request May 23, 2015
@MGotink MGotink merged commit 3ecc000 into metricfu:master May 23, 2015
@bf4 bf4 mentioned this pull request May 31, 2015
@bf4
Copy link
Member

bf4 commented Jun 18, 2015

I just pushed 4.12.0. I couldn't remember my password, so didn't sign it.. will bump a patch if I can remember.

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.

2 participants