Hello all, this is a topic that has come up a few times in the past, I believe I have a solution but I would like to get feedback from others
What we currently do
In Katello, we duplicate many packages from Foreman and the foreman-js meta packages. This is because we only npm install in Katello in our CI testing environment, and we don’t install packages from Foreman. The same may be true of other plugins.
The problem
This is an issue for two reasons:
In production and a development environment, the runtime dependencies come from Foreman, so what we are using for testing could be a different version than what is used in production. We are specifying the same dependency twice and can be hard to keep the versions in-sync
More time spent installing packages locally for developers since many are duplicated in Katello’s node_modules folder
In general we would like to Katello to function as a plugin and not an independent application
Proposed solution
We can use packages from Foreman with some changes to our jest configuration. We already use code from Foreman in the React tests with the changed resulting from the linked discussion. So it is easy to point jest (the React test runner) at Foreman’s node_modules.
So in CI this looks like:
checkout both Foreman and Katello
npm i in Foreman
Run tests in Katello
This will allow us to remove the duplicated packages from Katello.
I have a POC that can be done on a plain centos7 box, which is how I tested this method out. It uses this commit which updates the jest config to point to foreman’s node modules.
The changes that would have to happen:
Modify the jenkins job to checkout katello and foreman as siblings in a directory
side note on this: Is there any reason we checkout Foreman inside of Katello in CI today?
npm i only in Foreman (at least for the React portion of things) in the jenkins job
Make this update to Katello (will clean it up, it’s just a POC now) that updates the testing config and remove the duplicate packages.
I think this will be another step towards having our plugin ecosystem working well with the modern JS code we are using. It’s a pretty small change that we can implement to test Katello the same way we use it in a production environment and not duplicate packages.
Sounds like a good direction for me. a couple of questions:
Should we add a method that trigger all plugin tests from foreman core? Right now ruby tests for plugins can be triggered from core using e.g. rake test:foreman_ansible, and when plugins are installed rake test also triggers the plugin tests.
How will this work with JS CI that is mostly currently done on travis? Would those jobs need modification or migration into jenkins?
For Katello, we just run linting and I haven’t figured out how to not duplicate the linting packages so this shouldn’t be affected, the linting packages will stay for now and travis will keep working. Though I think the linting could probably just be done in Jenkins for Katello. I think in the future we can convert to foreman’s eslint plugin to make things easier and possibly remove the duplicate eslint packages.
For other plugins that do the React testing in Travis, the change should be able to be accommodated. I was actually testing this out in github actions and was making progress. If you can check out both the directories, npm install, and npm test, all should work as expected.
Side note: gh actions will likely not work for Katello because we have to npm install the packages in Jenkins to test assets and we also run the tests for Satellite which does not use github.
I also think the “find where foreman is checked out” logic that is part of Katello’s jest.config.js can be moved to it’s own npm package so other plugins can use it.
I would like to clarify: the proposed change will not break any plugins, no changes will have to be made to Foreman core for this to work, its logic on the plugin side to find foreman and use it’s node modules.
Thanks for sharing, perhaps it would, I would have to poke around more. I imagine we want to use https://www.npmjs.com/package/@theforeman/eslint-plugin-foreman in katello and standardize everything the same as Foreman best we can, but I still have to figure out how that works across repos.
Overall, I think it would be best to tackle the linting separately from the changes proposed here, linting packages are not runtime packages so it’s not as big of a concern as duplicating runtime packages. The changes proposed here may help make things easier, I definitely would like to not duplicate the linting packages and standardize more with Foreman in the not-too-distant future.
Thanks for bringing that up @John_Mitsch, that approach make sense.
Another way that might work is by installing foreman in katello as an npm module.
I recall myself experiencing with that a little bit locally by running npm install ../foreman in katello and configuring jest/storybook/lint to use foreman from node_modules.
This would keep the package versions in-sync, but would still duplicate all the packages from foreman right? Because they would be installed in foreman and in katello’s node_modules.
Well, if you would have "foreman": "../foreman", in your package.json and you would run npm i. Then, npm should only create a symlink.
If you don’t have foreman cloned locally, you might be able to get it (source and deps) from github with something like this in your package.json "foreman": "https://github.com/theforeman/foreman",.
Interesting, it does create the symlink as you said. The only thing to consider is always having foreman in the same location. The advantage of handling things in jest is that you can do more logic in the config file since it is in JavaScript vs. the package.json file which is JSON. Currently we search for foreman in these locations because in CI we checkout Foreman inside of Katello (I never did find out why we do that).
Though part of this RFC is to check out the Foreman and Katello repos as siblings, same as the development environment, so making that a standard across dev and CI environments is worth considering and we can used the fixed path.
I was able to use the Foreman dependencies and I used github actions for the react tests, which I am proposing we move the React tests to for Katello, assuming they work as expected.
I suspect it’s because the default way to check out a git repo in Jenkins is directly in the workspace. AFAIK there’s no need for this and can be changed.
I wanted to give some updates on this effort. In Katello we removed most of the duplicated packages and use them from Foreman during testing. We also added a non-blocking GH action for React tests that allows us to get feedback quickly on PRs. This happened in https://github.com/Katello/katello/pull/8722
I added a find-foreman package to the foreman-js repo here to find foreman in a relative directory in a testing environment. This can be used in .eslintrc.js and jest.config.js files to point linting and testing at Foreman’s modules instead of duplicating them, using the full path to ensure this works in both local and CI environments. You can see the katello PR that is switching to use the find-foreman package to see the areas we do this.
It feels good to not duplicate the packages and be able to use components directly from Foreman instead of mocking them during tests. If any Foreman plugins would like to do the same and have questions feel free to ask here or privately ping me
Are there any plugin examples that followed already?
I’d like to give it a try, but all the jest config in simplier plugin is coming from the tfm-test script from foreman-js.
Should we switch it there?
However, I would like to work this configuration into foreman-js itself so it works “out of the box” and plays nice with tfm-test. I plan to work on this soon and can give updates here.
Yes, the eslint rules were modified here, apologies for the breakages. This allows packages to be used from Foreman without breaking eslint extraneous dependency rules.
Next step will be making the packages available to Jest.
