-
Notifications
You must be signed in to change notification settings - Fork 0
Consistent Spec Setup #160
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
ce2e1b0 to
adf9f21
Compare
wesrich
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 think this PR is fine, other than the internal node package being dropped. Once that comment is solved, it otherwise seems like it should be fine :)
|
|
||
| TEST_COMMAND = 'NODE_ENV=test jp-runner --config jp-runner.config.mjs --webpack-config webpack.config.cjs' | ||
|
|
||
| # TODO: Yarn can't find this package: @rolemodel/jasmine-playwright-runner |
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.
https://github.com/RoleModel/jasmine-playwright-runner/pkgs/npm/jasmine-playwright-runner
It's an internal package that requires some config. Presumably anyone that's running this generator should have that key.
Maybe instead of removing this dependency, we could check for the presence of this key/ENV variable, and add it to the array if it's available? (Or maybe this generator raises if the key doesn't exist, since I think the whole point is to install that one dependency?)
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.
@wesrich. Would you mind pairing with me on implementing the check you're describing? I don't know what configuration I was expected to have in place and yarn only reported the package not being found.
I feel like the tests should confirm that it works, given the proper credentials.. But maybe it's not worth the GHA configuration that would require.. Opinion?
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.
@wesrich I couldn't find any solid documentation on what is required to access our private packages. I found a reference to ENV['GITHUB_PACKAGE_TOKEN'] in the L.CAD Academy repo, so that's my best guess and I've updated the generator accordingly.. 🤷♂️
Why?
There is a lot of boilerplate required to setup generator specs.
What Changed
What changed in this PR?
replace_contentGenerator helperPre-merge checklist
lib/rolemodel_rails/version.rbJasminePlaywrightGenerator issues:
@rolemodel/jasmine-playwright-runnerwithout a valid github package token.