-
Notifications
You must be signed in to change notification settings - Fork 7
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
add JS plugin test workflow #47
Conversation
843a0c7
to
fa9218a
Compare
The failures are legit and fixed in Katello/katello#10929 and theforeman/foreman#10095 |
a9e2371
to
93caebe
Compare
cc @dosas you might be interested :) |
@evgeni Thanks, very nice! |
exclude: ${{ inputs.matrix_exclude }} | ||
|
||
test: | ||
name: "Foreman ${{ inputs.foreman_version }} Ruby ${{ matrix.ruby }} and Node ${{ matrix.node }}" |
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.
Why do we need ruby for js tests?
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.
Foreman's postinstall script calls script/npm_install_plugins.js
https://github.com/theforeman/foreman/blob/4abf925fbd72264e8c316d720c03c1dd75688507/package.json#L19
And that, via script/plugin_webpack_directories.js
needs Ruby.
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.
Mind you, we just ensure Ruby is there, we do not install any gems or anything, so that step is rather quick.
(also, you only need foreman to be even there for some plugins that share code with Foreman via find-foreman
: https://github.com/Katello/katello/blob/master/webpack/.eslintrc.js, https://github.com/theforeman/foreman_openscap/blob/02d91220a206b6c121f6938a826eb9b4598af2f3/jest.config.js#L4)
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.
We could also skip installing a specific Ruby, given that GHA images come with Ruby pre-installed, but it felt wrong to me as that could lead to a version of Ruby that is unsupported by our stack.
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.
Thanks for the clarification
🍏 |
Documenting this in README would be nice. |
#48 adds docs |
Fixes: #42