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

set_code_dirs check for existence #129

Merged
merged 3 commits into from
Sep 25, 2013

Conversation

GeorgeErickson
Copy link

No description provided.

@@ -212,6 +212,7 @@ def load_metric(metric)
describe 'if #rails? is true ' do

before(:each) do
Dir.stub(:exists?).and_return(true)
Copy link
Member

Choose a reason for hiding this comment

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

Is there, perhaps, a better way to do this? Maybe temporarily set the code_dirs yourself? Something less library-level global

BTW, I noticed that a number of tests fail when I touch config/environment.rb as our tests apparently don't mock the code_dirs properly somewhere. I also don't see anything terrible that happens when running rbp on a non-rails project... but that's off topic.

Copy link
Author

Choose a reason for hiding this comment

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

I should probably be less heavy handed with this stub. And only return fake the existence of an app directory.

Copy link
Member

Choose a reason for hiding this comment

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

If you think you can, that'd be great. Side-effects of one test leaking into another are hard to find and fix. (And I know there's already stuff like this in the test suite, but I want to remove that, too!)

@ghost ghost assigned bf4 Aug 21, 2013
@bf4
Copy link
Member

bf4 commented Aug 21, 2013

Thanks for this!

@bf4 bf4 mentioned this pull request Aug 21, 2013
58 tasks
@bf4
Copy link
Member

bf4 commented Aug 29, 2013

Want help finishing this PR?

@bf4
Copy link
Member

bf4 commented Sep 10, 2013

@GeorgeErickson hi

@bf4
Copy link
Member

bf4 commented Sep 20, 2013

@GeorgeErickson Want help?

@bf4 bf4 merged commit d0fb778 into metricfu:master Sep 25, 2013
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.

None yet

2 participants