Add js tests #75
Add js tests #75
Conversation
rahulsom
force-pushed
the
add-js-tests
branch
3 times, most recently
from
7753820 to
072e2f6
Compare
rahulsom
force-pushed
the
add-js-tests
branch
2 times, most recently
from
7aa5787 to
660a418
Compare
| "jquery": "^3.7.0", | ||
| "@types/jquery": "^3.5.16" | ||
| }, | ||
| "dependencies": { |
There was a problem hiding this comment.
It will be nice if someday Jenkins move to having a separate UI layer, so that we can declare dependencies to plug-ins and other libs here (like the JQuery one). Although that's probably hard with stapler and all the plugin tooling.
rahulsom
force-pushed
the
add-js-tests
branch
5 times, most recently
from
785b153 to
3a10e74
Compare
This extracts some code out to `Util.ts` and tests it using jest in `Util.test.ts`. `unochoice.js` has been renamed to `UnoChoice.es6` so we can synthesize `UnoChoice.js`.
|
Ready for review @rahulsom ? |
|
Yeah. |
| @@ -8,6 +8,7 @@ | |||
| 4. [#71](https://github.com/jenkinsci/active-choices-plugin/pull/71): test: Add end-to-end tests for UI behavior, thanks to @rahulsom | |||
| 5. [#74](https://github.com/jenkinsci/active-choices-plugin/pull/74): refactor: Extract helper functions and drop table-layout support, thanks @rahulsom | |||
| 6. [#78](https://github.com/jenkinsci/active-choices-plugin/pull/78): Refresh plugin for June 2023, thanks @basil | |||
| 7. [#75](https://github.com/jenkinsci/active-choices-plugin/pull/75) Add test for pure javascript code, thanks @rahulsom | |||
There was a problem hiding this comment.
A few random comments, but nothing major. My main concern was having any build files in the source directories, but that's not an issue here! So it's looking really good!
I abandoned webpack in the Vue projects I use at $work and Apache, and have been using Vite/vitest/etc. But webpack + jest looks fast enough, so no need to bother with that.
Thanks @rahulsom !
| @@ -965,3 +829,4 @@ var UnoChoice = UnoChoice || ($ => { | |||
| instance.cascadeParameters = cascadeParameters; | |||
| return instance; | |||
| })(jQuery3); | |||
| window.UnoChoice = UnoChoice; | |||
There was a problem hiding this comment.
Isn't there a way to use webpack to avoid polluting window? Like merging/combining the files, maybe minifying too (we already have the source map anyway).
There was a problem hiding this comment.
We need to call the functions from the jelly files. So it needs to be exposed at the moment. I'm guessing if there's a way to have the module search for components on the page and decide to run the code, we can avoid polluting window.
The way it's written, this behavior is mimicking what we had before.
There was a problem hiding this comment.
Fair enough 👍 I think you've answered everything, LGTM
There was a problem hiding this comment.
This new module looks perfect! 🙆♂️ !
|
Thank you @rahulsom 🙇 |
This extracts some code out to
Util.tsand tests it using jest inUtil.test.ts.unochoice.jshas been renamed toUnoChoice.es6so we can synthesizeUnoChoice.js.Testing done
This PR adds tests on top of #74
Submitter checklist