test: add unit tests + CI enforcement (Phases 1-4) - #50
Merged
Conversation
…kField Establishes the Vitest + Vue Test Utils toolchain end-to-end (already installed but never exercised - no test files existed anywhere in the repo) and adds a shared tests/unit/testUtils.js mount helper covering the $log global and FontAwesomeIcon/BFormDatepicker stubs. Also fixes eslint.config.mjs's test-file block, which only granted Node globals and would otherwise flag describe/it/expect as no-undef under vitest's config.globals: true. Along the way, the tests surfaced two pre-existing issues worth a closer look separately: - LinkField.vue's <b-form-datepicker> has no matching component in bootstrap-vue-next (only existed in the old Vue2 bootstrap-vue) - it silently fails to render, same migration-gap pattern as the BTable filter API and navbar dark-theme issues. - SelectField.vue's `selected` prop has no default, so an omitted `selected` resolves to undefined, and created()'s `this.selected !== null` check fires unconditionally on every mount that doesn't explicitly pass `selected: null`, emitting update:modelValue with undefined. Locked in by a dedicated test rather than fixed, since fixing it is out of scope here.
Adds @pinia/testing and establishes the vi.mock('@/services/httpclient')
pattern needed for every API-calling unit under test - the real module
imports the Keycloak singleton at load time, so mocking httpclient.js
directly (rather than keycloak.js separately) keeps the real client from
ever instantiating.
Covers all 6 src/mixins/ files plus TagInput.vue (httpClient +
useComicListStore via createTestingPinia), 33 new tests.
Also adds tests/unit/setup.js (wired via vite.config.js test.setupFiles)
stubbing ResizeObserver, which jsdom doesn't implement but bootstrap-vue-next's
dropdown positioning (@floating-ui, used by TagInput.vue's b-dropdown) needs.
Surfaced two more pre-existing bugs while writing these, locked in via
dedicated tests rather than fixed here:
- roleservice.js's addRoleOption() sets `roleOption.text = name` (a bare
global reference) instead of `role.name`, so every role option's text
is empty rather than the role's actual name.
- textservice.js's saveText()/deleteText() reference `this.loading`,
which the mixin itself never declares in data() - relies entirely on
the host component providing it (works today because every current
host does, but it's an implicit contract worth knowing about).
sundancekid73
changed the title
Tests ComicList.vue - the most complex component, combining the Pinia/httpClient mocking pattern from Phase 2 with a $router mock and fake timers for its debounced search. Extends testUtils.js's mountOptions() with the $statusOptions/$typeOptions/moment global property mocks it needs (registered as globalProperties in main.js, same as $log). Uses shallowMount rather than mount to avoid pulling in bootstrap-vue-next's full b-table/b-pagination/b-collapse tree, since the coverage target here is the component's own methods/computeds, not re-testing library internals. Covers: loadComicList, the mounted() browseMode/searchTerm branching, filteredComics (status/type/text filtering), typeAbbreviation, fullName, edit (router navigation), deleteComic, promptDelete/ confirmDelete, and clearSearchTermAndFilter. 21 tests. Also adds tests/unit/stores/comicListStore.spec.js covering the store's default state shape and reactivity, now that it's exercised indirectly through ComicList/TagInput tests too.
sundancekid73
changed the title
…reen Two independent, narrowly-scoped fixes surfaced by writing this PR's tests and about to be enforced by the new CI lint step: - KeywordList/PersonList/PredicateList/PublisherList/RolesList.vue each declared totalRows twice: once as a static data() field (totalRows: 1) and again as a computed deriving it from the filtered list length. The data() field is leftover from before the BTable filter fix (4631290) added the computed - removing it resolves vue/no-dupe-keys. - TagInput.vue destructured an unused `tags` binding from the b-form-tags default slot. No behavior change in either case.
No secrets needed, so unlike docker-image-stage.yml this doesn't need a dependabot[bot] actor guard - it should (and does) run for everyone, including Dependabot PRs. Landed after Phases 1-3 gave the suite enough substance (74 tests) for a red check to mean something, and after fixing the 6 pre-existing lint errors so the gate starts green rather than red.
sundancekid73
changed the title
… name bugs SelectField.vue: `selected` had no default, so an omitted prop resolved to undefined rather than null, and created()'s `this.selected !== null` check fired unconditionally on every mount - emitting update:modelValue with undefined and clobbering the parent's value. Fixed by giving `selected` an explicit `default: null`. roleservice.js: addRoleOption() set `roleOption.text = name`, a bare identifier that resolved to the global `window.name` instead of the intended `role.name`, so every role dropdown option's text was blank. Updates the two tests (added earlier in this branch) that had locked in the old buggy behavior to assert the fixed behavior instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phases 1-4 of a plan to add unit tests to admin-webapp, which had zero tests and no CI test enforcement despite vitest/@vue/test-utils/jsdom already being installed and configured.
Phase 1 — toolchain proof
tests/unit/testUtils.jssharedmountOptions()helper.InputField.vue,SelectField.vue,LinkField.vue.eslint.config.mjs's test-file block (missing vitest globals).Phase 2 — API/Pinia mocking pattern
@pinia/testing; establishesvi.mock('@/services/httpclient')as the mock point for every API-calling unit (avoids transitively instantiating the real Keycloak client).src/mixins/, via a newmountMixin()helper, plusTagInput.spec.js(httpClient +createTestingPinia).tests/unit/setup.jsstubbingResizeObserver(jsdom gap bootstrap-vue-next's dropdown needs).Phase 3 — heaviest tier
ComicList.spec.js(21 tests) — the most complex component, combining the Pinia/httpClient pattern with a$routermock and fake timers for its debounced search.comicListStore.spec.js— default state shape and reactivity.Phase 4 — CI enforcement
.github/workflows/test.yml, runningpnpm lint+pnpm test:uniton every PR and push tomain. No secrets needed, so (unlikedocker-image-stage.yml) it runs for everyone including Dependabot PRs.KeywordList/PersonList/PredicateList/PublisherList/RolesList.vueeach declaredtotalRowstwice (a staticdata()field left over from before the BTable filter fix added a computed of the same name).TagInput.vuedestructured an unusedtagsbinding.74 tests total, all passing. CI verified green on this PR.
Bugs surfaced by writing these tests — fixed
SelectField.vue:selectedprop had no default, so an omittedselectedresolved toundefinedrather thannull, andcreated()'sthis.selected !== nullcheck fired unconditionally on every mount — emittingupdate:modelValuewithundefinedand clobbering the parent's value. Fixed with an explicitdefault: null.roleservice.js:addRoleOption()setroleOption.text = name— a bare identifier resolving to the globalwindow.nameinstead of the intendedrole.name— so every role dropdown option's text was blank. Fixed torole.name.The two tests that had locked in the old buggy behavior were updated to assert the corrected behavior instead.
Remaining known issue (not fixed here, flagged for follow-up)
LinkField.vue's<b-form-datepicker>doesn't exist in bootstrap-vue-next — silently renders nothing in production. This is a missing-component gap (same pattern as the earlier BTable-filter and navbar-theme migration bugs), not a small logic fix, so left for a separate PR.textservice.js'ssaveText()/deleteText()rely onthis.loading, which the mixin itself never declares indata()— works today only because every current host component happens to provide it. Noted as an implicit contract, not fixed (no observable bug today).This completes the plan's 4 phases plus the two bug fixes. Natural follow-up (not in this PR): repeat the
ComicList.vuetest pattern for the other 5 List components, fix theLinkField.vuedate picker gap, and optionally add coverage reporting once there's a bigger baseline.Test plan
pnpm test:unit— 74/74 tests passpnpm lint— 0 errorstestworkflow verified green on this PR🤖 Generated with Claude Code