Skip to content

fix(tests,lint): two stale specs pinned removed contracts; arm the Vue 3 lint gate - #100

Merged
rubenvdlinde merged 1 commit into
developmentfrom
fix/unit-test-and-eslint-preset
Aug 2, 2026
Merged

fix(tests,lint): two stale specs pinned removed contracts; arm the Vue 3 lint gate#100
rubenvdlinde merged 1 commit into
developmentfrom
fix/unit-test-and-eslint-preset

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Unit failures — both were stale specs, not flake

Baseline on the unmodified base at load ~3 was 2 failures, not 1:

 FAIL tests/vitest/SaveAsTemplateAction.spec.js > … > openSaveAsTemplate gathers schemas + templates and opens the dialog
 FAIL tests/vitest/views/ExportJobsList.spec.js  > … > filters by the applicationSlug prop

Both are tests still asserting behaviour that a later, correct production fix deliberately replaced. Neither could be noticed, because no app's JS unit suite has ever run in CI (quality.yml's frontend-checks defaults to "[]" and the job is guarded on != '[]').

1. SaveAsTemplateAction — the mock was call-order dependent

The spec queued one axiosMock.get.mockResolvedValueOnce(...) for the templates read. openSaveAsTemplate() now issues two GETs — it resolves the manifest from /api/applications/{slug}/manifest first, then reads templates. The one-shot response was consumed by the manifest call; the templates read fell through to the generic { data: application } default, which has no results array → existingTemplates came back [].

Production is right. The manifest GET was added because an Application record carries neither manifest nor currentVersion, so the old obApp.manifest read always fell through to {} and saving any app as a template was impossible. The mock is now routed by URL, removing the ordering dependency and letting both responses be asserted; the manifest endpoint is now asserted explicitly too.

2. ExportJobsList — the spec pinned the exact bug that was fixed

It asserted filter[applicationSlug]=my-app. Commit c1d22f4f5 (#95) replaced that with a plain ?applicationUuid=, for two independently sufficient reasons measured against real stored jobs: export-job declares no applicationSlug property at all, and the filter[...] bracket syntax is not what the endpoint reads. Production is right; the expectation is corrected, and both ruled-out shapes are now pinned negatively so neither can come back.

No assertion was weakened, skipped, or given a longer timeout.

Lint — adopt @conduction/nextcloud-vue/eslint

eslint --print-config on this Vue 3 app showed zero vue/no-deprecated-* rules armed and vue/no-multiple-template-root armed at [2]. Spreading conductionVue3Fixes last arms 21 deprecation rules and disarms the three inverted Vue-2 rules (the two hand-rolled local disables are now redundant and were removed).

Positive control — because "0 deprecation findings" is meaningless without one. Injecting a beforeDestroy() hook into PageDesigner.vue:

config result
with shared preset error … vue/no-deprecated-destroyed-lifecycle
base config (before) completely silent

That is the same configuration that let four beforeDestroy hooks survive openconnector's Vue 3 migration as silent memory leaks. Sentinel reverted.

Backlog: 9 findings, all fixed — zero deferred

5 × vue/v-on-event-hyphenation, 4 × comma-dangle.

The hyphenation fixes were applied by hand, never via --fix, and only after proving they are semantics-preserving:

<C @update:dataSource="f" />   -> "onUpdate:dataSource"
<C @update:data-source="f" />  -> "onUpdate:dataSource"

@vue/compiler-sfc compiles both spellings to the identical handler key (Vue 3 camelizes a static v-on argument). The config comment removed in this PR asserted the opposite — that the hyphenated form is "silently DEAD" — and that claim is false. All four edited components have their own passing specs.

Positive control for the 0/0 lint baseline: an injected var + unused binding reports 2 errors, so the clean baseline is real and not a misconfigured lint run.

After

140 files / 1364 tests pass, twice, at load ~3-4. npm run lint: 0 errors, 0 warnings.

…e 3 lint gate

Both unit failures were tests still asserting behaviour that a later, correct
production fix had deliberately replaced. Neither could be noticed, because
no app's JS unit suite has ever run in CI — the shared `quality.yml`
`frontend-checks` input defaults to `"[]"` and the job is guarded on
`!= '[]'`, so the matrix always resolved empty and reported "skipping".

Baseline on the unmodified base at load ~3 was 2 failures, not 1:

  SaveAsTemplateAction > openSaveAsTemplate gathers schemas + templates …
  views/ExportJobsList > filters by the applicationSlug prop

1. SaveAsTemplateAction — the mock was call-ORDER dependent.

   The spec queued one `axiosMock.get.mockResolvedValueOnce(...)` for the
   templates read. `openSaveAsTemplate()` now issues TWO GETs: it resolves
   the manifest from `/api/applications/{slug}/manifest` FIRST, then reads
   the templates. The one-shot response was therefore consumed by the
   manifest call, and the templates read fell through to the generic
   `{ data: application }` default — which has no `results` array, so
   `existingTemplates` came back `[]`.

   Production is right. The manifest GET was added because an Application
   record carries neither `manifest` nor `currentVersion`, so the old
   `obApp.manifest` read always fell through to `{}` and saving ANY app as
   a template was impossible. The mock is now routed by URL, which removes
   the ordering dependency and lets both responses be asserted; the
   manifest endpoint is now asserted explicitly too.

2. ExportJobsList — the spec pinned the exact bug that was fixed.

   It asserted `filter[applicationSlug]=my-app`. Commit c1d22f4 (#95)
   replaced that with a plain `?applicationUuid=`, for two independently
   sufficient reasons measured against real stored jobs: `export-job`
   declares no `applicationSlug` property at all, and the `filter[...]`
   bracket syntax is not what the endpoint reads. Production is right; the
   expectation is corrected, and both ruled-out shapes are now pinned
   negatively so neither can come back.

No assertion was weakened, skipped, or given a longer timeout.

LINT — adopt `@conduction/nextcloud-vue/eslint`

`eslint --print-config` on this Vue 3 app showed ZERO `vue/no-deprecated-*`
rules armed and `vue/no-multiple-template-root` armed at `[2]`. Spreading
`conductionVue3Fixes` last arms 21 deprecation rules and disarms the three
inverted Vue-2 rules (the two hand-rolled local disables are now redundant
and were removed).

Positive control, because "0 deprecation findings" is meaningless without
one: injecting a `beforeDestroy()` hook into PageDesigner.vue errors with
the shared preset and is COMPLETELY SILENT on the base config — the same
configuration that let four `beforeDestroy` hooks survive openconnector's
Vue 3 migration as silent memory leaks. Sentinel reverted.

The preset surfaced 9 findings, all fixed (backlog is now zero, not
deferred): 5 `vue/v-on-event-hyphenation` + 4 `comma-dangle`.

The hyphenation fixes were applied by hand, never via `--fix`, and only
after proving they are semantics-preserving. `@vue/compiler-sfc` compiles
BOTH `@update:dataSource` and `@update:data-source` to the identical
handler key `"onUpdate:dataSource"` (Vue 3 camelizes a static v-on
argument). The removed config comment asserted the opposite — that the
hyphenated form is "silently DEAD" — and that claim is false. The four
edited components each have their own passing specs.

Verified by deliberate break (both reverted):
- templates response -> SENTINEL: reds SaveAsTemplateAction
- applicationUuid prop -> SENTINEL: reds ExportJobsList
Positive control for the 0/0 lint baseline: an injected `var` + unused
binding reports 2 errors, so the clean baseline is real and not a
misconfigured lint run.

After: 140 files / 1364 tests pass, twice, at load ~3-4.
`npm run lint`: 0 errors, 0 warnings.
@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openbuild @ b6c3f5d

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
composer ✅ 100/100
npm ✅ 662/662
PHPUnit
Newman ⏭️
Playwright ⏭️

Coverage: 89.4% (17/19 statements)


Quality workflow — 2026-08-02 20:09 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit 4b82684 into development Aug 2, 2026
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants