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

Make spec testing placeholders in string a shared example #13514

Conversation

mzazrivec
Copy link
Contributor

All the plugins we have (UI Classic, Amazon provider, etc.) would then run this shared example
as a part of their CI suite. This way, the spec doesn't have to test (and potentially fail) all loaded
plugins.

@@ -23,6 +23,7 @@
Dir[Rails.root.join("spec/support/**/*.rb")].each { |f| require f }
# include the manageiq-gems-pending matchers
Dir[ManageIQ::Gems::Pending.root.join("spec/support/custom_matchers/*.rb")].each { |f| require f }
Dir[Rails.root.join("spec/shared/**/*.rb")].each { |f| require f }
Copy link
Member

Choose a reason for hiding this comment

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

Is this really necessary? There's already a /shared with models/floating_ip.rb. How is that currently required now? (or is it unused?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ugh, you're right, this is not needed. Instead of putting it just into ManageIQ/manageiq-ui-classic#164 I mistakenly put it both into that & this PR. Thanks for noticing that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, requiring just spec/shared/i18n/*.rb should be enough.

@mzazrivec mzazrivec force-pushed the string_placeholders_spec_as_a_shared_example branch 2 times, most recently from a8a55fa to 496cf4d Compare January 18, 2017 11:56
@@ -23,6 +23,7 @@
Dir[Rails.root.join("spec/support/**/*.rb")].each { |f| require f }
# include the manageiq-gems-pending matchers
Dir[ManageIQ::Gems::Pending.root.join("spec/support/custom_matchers/*.rb")].each { |f| require f }
Dir[Rails.root.join("spec/shared/i18n/*.rb")].each { |f| require f }
Copy link
Member

Choose a reason for hiding this comment

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

Upon reviewing this more and seeing the origin of /shared (#11414) I think your original line was correct. Let's change this back to just /shared/**/*.rb and place it right below line 23 (the support one). Then all shared examples here will be available to the core specs (and we'll start consolidating them there, more.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chrisarcand All right, done.

@mzazrivec mzazrivec force-pushed the string_placeholders_spec_as_a_shared_example branch from 496cf4d to 2892e6c Compare January 18, 2017 15:38
@miq-bot
Copy link
Member

miq-bot commented Jan 18, 2017

Some comments on commit mzazrivec@2892e6c

spec/shared/i18n/placeholders_spec.rb

  • ⚠️ - 26 - Detected puts. Remove all debugging statements.
  • ⚠️ - 28 - Detected puts. Remove all debugging statements.
  • ⚠️ - 30 - Detected puts. Remove all debugging statements.

@miq-bot
Copy link
Member

miq-bot commented Jan 18, 2017

Checked commit mzazrivec@2892e6c with ruby 2.2.6, rubocop 0.46.0, and haml-lint 0.19.0
3 files checked, 0 offenses detected
Everything looks good. ⭐

@chrisarcand chrisarcand merged commit 00ae1bb into ManageIQ:master Jan 18, 2017
@chrisarcand chrisarcand added this to the Sprint 53 Ending Jan 30, 2017 milestone Jan 18, 2017
@mzazrivec mzazrivec deleted the string_placeholders_spec_as_a_shared_example branch February 16, 2017 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants