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 vmware spec tests #2

Merged
merged 2 commits into from
Jan 24, 2017
Merged

Conversation

agrare
Copy link
Member

@agrare agrare commented Jan 18, 2017

Missed the big comment that said # Uncomment in case you use vcr cassettes and requires with Rails.root are no longer working so use the Engine.root

@agrare
Copy link
Member Author

agrare commented Jan 18, 2017

@durandom @Fryguy please review

Copy link
Member

@durandom durandom left a comment

Choose a reason for hiding this comment

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

spec/.spec_helper.rb.swp added by accident

about the require I have no strong opinion, up to you.

otherwise, good to go

@@ -1,4 +1,4 @@
require Rails.root.join('spec/tools/vim_data/vim_data_test_helper')
require File.join(ManageIQ::Providers::Vmware::Engine.root, 'spec/tools/vim_data/vim_data_test_helper.rb')
Copy link
Member

@durandom durandom Jan 18, 2017

Choose a reason for hiding this comment

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

You can also

require ManageIQ::Providers::Vmware::Engine.root.join('spec/tools/vim_data/vim_data_test_helper.rb').to_s

or, what I do in aws, require_relative and keep the file close to where it's actually used.
Like vim_data_test_helper seems to be only used by specs in vmware/infra_manager - why not put it here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I like the idea of moving it, will push a commit for that shortly

@@ -3,10 +3,9 @@
SimpleCov.start
end

# Uncomment in case you use vcr cassettes
Copy link
Member

Choose a reason for hiding this comment

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

oops, missed that. sorry

@agrare
Copy link
Member Author

agrare commented Jan 18, 2017

spec/.spec_helper.rb.swp added by accident

Ah how did that sneak in :( good 👀 fixed

@agrare
Copy link
Member Author

agrare commented Jan 18, 2017

Okay moved spec/tools/vim_data to spec/models/manageiq/providers/vmware/infra_manager and updated the spec tests.

Copy link
Member

@durandom durandom left a comment

Choose a reason for hiding this comment

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

LGTM, cant merge though

@agrare
Copy link
Member Author

agrare commented Jan 23, 2017

Moving the vim_data files was a lot of change so went with @durandom's other suggestion

This gets the tests green, @Fryguy @blomquisg merge?

@blomquisg blomquisg merged commit 9a3097d into ManageIQ:master Jan 24, 2017
@agrare agrare deleted the fix_vmware_spec_tests branch January 24, 2017 14:31
@agrare agrare added this to the Sprint 54 Ending Feb 13, 2017 milestone Jun 5, 2018
agrare pushed a commit to agrare/manageiq-providers-vmware that referenced this pull request Apr 15, 2019
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

4 participants