always execute access_permissions_test from Foreman core

[evgeni]

No description provided.

[ekohl]

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:

https://github.com/theforeman/foreman_openscap/blob/09d1197ee823cfa7d1425f6498ba0686dcbf8a12/lib/tasks/foreman_openscap_tasks.rake#L95-L107

I'm still leaning to making sure rake test:$plugin does the right thing, because that's what developers should also run locally.

[evgeni]

So, we could have a test:things_plugins_need_to_test_from_core_and_naming_is_hard that lists access_permissions and test:$plugin depends on that (essentially, what openscap is doing plus an dependency)?

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?

[ekohl]

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

[kamils-iRonin]

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.

[evgeni]

I am going to call theforeman/foreman_plugin_template#51 the correct implementation for now, closing.

[nadjaheitmann]

Actually, I would prefer to re-open this PR compared to the one in the plugin template. I think that all plugins should run this test, it is important. So why not just unify it here?

@evgeni @ekohl @kamils-iRonin Any opinions?

I don't see the permissions test executed very often in CI. Not sure what is the reason. It probably got lost somehow as the CIs switched to using this action.

[evgeni]

I still like @ekohl's statement:

I'm still leaning to making sure rake test:$plugin does the right thing, because that's what developers should also run locally.

Granted, as nobody extends test:$plugin today, it's also not true (as you have rightfully pointed out).

Let's see what #91 says while I think of how this can be done smarter.

[nadjaheitmann]

Granted, as nobody extends test:$plugin today, it's also not true (as you have rightfully pointed out).

Unfortunately.

Developers can also run the permissions test locally from Foreman directory, but they don't do that either. I see myself relying a lot on upstream CI as well.

[nadjaheitmann]

Somehow related topic: I also see many plugins referencing v0 instead of v1. I assume some updates are required there as well?

[evgeni]

Somehow related topic: I also see many plugins referencing v0 instead of v1. I assume some updates are required there as well?

Don't these plugins utilize dependabot to update their GHA actions? ;)

[nadjaheitmann]

Somehow related topic: I also see many plugins referencing v0 instead of v1. I assume some updates are required there as well?

Don't these plugins utilize dependabot to update their GHA actions? ;)

DependaWho? ;)

https://github.com/theforeman/foreman_ansible/blob/master/.github/workflows/ruby_tests.yml#L25

I admit that this is not updated in various plugins that I am maintaining, either 🙈

[evgeni]

markt-de/foreman_ovirt@9e552af

Which then does things like markt-de/foreman_ovirt#18

[nadjaheitmann]

markt-de/foreman_ovirt@9e552af

Which then does things like markt-de/foreman_ovirt#18

I guess I'll open some PRs eventually, thanks!