-
Notifications
You must be signed in to change notification settings - Fork 8
always execute access_permissions_test from Foreman core #16
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
Conversation
ekohl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to avoid this and looked at patching plugins. Now that I look further I see a common pattern in some plugins to include from core.
https://github.com/theforeman/foreman_puppet/blob/master/test/unit/foreman_puppet/access_permissions_test.rb
https://github.com/theforeman/foreman_statistics/blob/master/test/unit/foreman_statistics/access_permissions_test.rb
https://github.com/theforeman/foreman_rh_cloud/blob/foreman_3_9/test/unit/rh_cloud_permissions_test.rb
https://github.com/theforeman/foreman_bootdisk/blob/master/test/unit/access_permissions_test.rb
Then foreman_openscap has this surprising bit:
I'm still leaning to making sure rake test:$plugin does the right thing, because that's what developers should also run locally.
|
So, we could have a the other linked things are good own tests (as in: they are testing that the routes done by the plugin are sane), but I think executing the core version of the test also ensure we don't break stuff from core? |
|
So I found how I tried to solve it in the past: theforeman/foreman-tasks@3ad347b can be summed up as theforeman/foreman_plugin_template#51 |
|
I like the idea of having a static list of recommended core tests in the core. I would also run all core tests on merge to master – to make sure that a plugin does not affect the core. Of course, making these core tests optional would still give us some flexibility. |
|
I am going to call theforeman/foreman_plugin_template#51 the correct implementation for now, closing. |
No description provided.