Skip to content

🧪 Add JS unit test setup, covering Util.js - #1989

Open
AndyScherzinger wants to merge 1 commit into
mainfrom
test/noid/js-unit-tests
Open

🧪 Add JS unit test setup, covering Util.js#1989
AndyScherzinger wants to merge 1 commit into
mainfrom
test/noid/js-unit-tests

Conversation

@AndyScherzinger

@AndyScherzinger AndyScherzinger commented Aug 17, 2026

Copy link
Copy Markdown
Member

The repo had no JS test runner. The only way to exercise frontend logic was Playwright - a Docker container, a login and a browser to check what a helper returns. This adds vitest and puts it to work on src/Util.js, which has been there all along, is pure, and had no coverage at all.

What is in here

  • vitest in jsdom, with npm run test and npm run test:coverage, and coverage/ ignored.
  • .github/workflows/node-test.yml, added verbatim from nextcloud/.github like the sixteen others already in .github/workflows. It runs the suite and uploads coverage to Codecov.
  • src/tests/Util.spec.js, 33 cases.

Why Util.js

It is on main already, so this lands without waiting on a feature branch, and every feature branch can then bring the tests for whatever it adds. escapeHtml() in particular is the one piece of that file where a mistake is a security bug, and it had no test.

Covered: copyNote's attribute list and its exclusions, the category label, the new-note route check, and the two drag helpers down to their defensive paths - a browser that refuses a mime type, a transfer that advertises no types, a read-only or unknown note. Util.js ends up fully covered by line and by branch.

Implementation notes
  • The config is deliberately smaller than the one in text, circles, deck or files_mindmap: no @vitejs/plugin-vue, since no component is tested yet.
  • jsdom rather than node, because escapeHtml() builds an element and because the app logger other modules pull in reads window at import time. A component test would want jsdom anyway.
  • The npm script is test rather than test:unit because the organisation's node-test.yml runs npm run test --if-present and npm run test:coverage --if-present. test:e2e keeps its name, so the two suites stay distinguishable.
  • The config file is vitest.config.js rather than .mjs, so it sits with the repo's other configs and inside that workflow's **.js paths filter - as .mjs, a config-only change would skip the test job.
  • Excluded attributes are asserted with toStrictEqual, since toEqual cannot tell a property set to undefined from one that was never assigned - which is exactly what copyNote() does with them.

Merge order

#1972 and #1974 each carry a spec for the module they add, and those cannot run until this got merged.

🤖 AI (if applicable)

  • The content of this PR was partly or fully generated using AI

@codecov

codecov Bot commented Aug 17, 2026

Copy link
Copy Markdown

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@AndyScherzinger
AndyScherzinger force-pushed the test/noid/js-unit-tests branch from 47ec9fb to 63b62e5 Compare August 17, 2026 21:00
@AndyScherzinger AndyScherzinger changed the title Test/noid/js unit tests 🧪 Add a JS unit test setup, covering Util.js Aug 17, 2026
@AndyScherzinger
AndyScherzinger changed the base branch from feat/noid/note-info to main August 17, 2026 21:07
@AndyScherzinger AndyScherzinger changed the title 🧪 Add a JS unit test setup, covering Util.js 🧪 Add JS unit test setup, covering Util.js Aug 17, 2026
@AndyScherzinger AndyScherzinger added this to the 6.1.0 milestone Aug 17, 2026
@AndyScherzinger AndyScherzinger added the AI assisted This PR contains AI-assisted commits label Aug 17, 2026
@AndyScherzinger
AndyScherzinger marked this pull request as ready for review August 17, 2026 21:12
@AndyScherzinger
AndyScherzinger force-pushed the test/noid/js-unit-tests branch from 63b62e5 to 0f5c0b1 Compare August 18, 2026 07:44
@AndyScherzinger
AndyScherzinger requested a lite review from Copilot August 18, 2026 07:45
@AndyScherzinger AndyScherzinger self-assigned this Aug 18, 2026

This comment was marked as outdated.

The repo had no JS test runner, so the only way to exercise frontend logic was
Playwright — a Docker container, a login and a browser to check what a helper
returns. Util.js has been there all along, is pure, and had no coverage at all.

vitest, as in text, circles, deck and files_mindmap. The config is deliberately
smaller than theirs: no @vitejs/plugin-vue, since no component is tested yet.
jsdom rather than node, because escapeHtml() builds an element and because the
app logger other modules pull in reads window at import time — a component test
would want jsdom anyway.

The config file is vitest.config.js rather than .mjs, so it sits with the
repo's other configs and inside the workflow's `**.js` paths filter — as
`.mjs` a config-only change would skip the test job.

The npm script is `test` rather than `test:unit` because the organisation's
node-test.yml workflow runs `npm run test --if-present` and
`npm run test:coverage --if-present`. That workflow is added verbatim from
nextcloud/.github, like the sixteen others already in .github/workflows.
`test:e2e` keeps its name, so the two suites stay distinguishable.

Util.js is the subject rather than anything newer on purpose: it is on main
already, so this lands without waiting on a feature branch, and every feature
branch can then bring the tests for whatever it adds.

Covered: copyNote's attribute list and its exclusions, the category label,
the new-note route check, and the two drag helpers down to their defensive
paths — a browser that refuses a mime type, a transfer that advertises no
types, a read-only or unknown note. escapeHtml gets its own cases, having been
the one piece of this file where a mistake would be a security bug.

33 cases, and Util.js fully covered by line and by branch.

Assisted-by: Claude Code:claude-opus-5[1m]
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
@AndyScherzinger
AndyScherzinger force-pushed the test/noid/js-unit-tests branch from 0f5c0b1 to 9385741 Compare August 18, 2026 09:48
@AndyScherzinger
AndyScherzinger requested a lite review from Copilot August 18, 2026 09:52

This comment was marked as low quality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review AI assisted This PR contains AI-assisted commits

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants