Improve test coverage and change test suite to vitest by Michiel-VandeVelde · Pull Request #220 · LinkedDataFragments/Server.js

Michiel-VandeVelde · 2026-08-21T12:15:20Z

No description provided.

Splits the Mocha->Vitest and JS->TS conversions into two steps so each diff stays small and reviewable. This is step 1: syntax only, no types.

…PatternFragmentsHtmlView feature-webid/controllers: 16% -> 80%, including a second pre-existing bug found along the way (not fixed): _verifyWebID's predicate switch compares an N3 term object to string literals with ===, so it never matches. feature-summary/views/summary: 18% -> 100%. QuadPatternFragmentsHtmlView.ts: 54% -> ~100%.

Util.ts: 87% -> 100% statements. UrlData.ts: found a third pre-existing bug along the way (not fixed): options.assetsPath is always ignored, since baseURLPath + 'assets/' is never an empty string and short-circuits the || fallback.

EmptyDatasource/IndexDatasource: 50%/26% -> ~100%. View.ts/ViewCollection.ts: cover the view-extension rendering paths. Error/Forbidden/NotFound HtmlView: cover their template-selection logic. HtmlView.ts: 35% -> 79%. The remaining gap is qejs's actual render call, which can't be exercised here: qejs resolves template paths via require.main.filename, which Vitest's module runner never sets, so every real qejs.renderFile call fails structurally under this test setup. Only the resulting error-handling path is covered; the success path isn't reachable in tests.

… feature-webid Datasource.ts: 96% -> 98.5% (graph-side blank-node translation, forced default graph without quad support). SparqlDatasource.ts: 92% -> 100% (_encodeObject/_convertLiteral edge cases). QuadPatternFragmentsController.ts: 88% -> 96.5% (extension chain, including the error-logging path, and close()). WebIDControllerExtension.ts: 80% -> 90% (the full HTTPS + TLS _handleRequest flow). The remaining gap is the predicate switch in _verifyWebID, already established as unreachable dead code.

…nches (96.6% -> 100%)

…rse-error noise

CliRunner.ts: 90.47% -> 98.41% (SIGHUP abort-before-listening and already-in-progress paths). The one remaining line is a defensive "workers.pop() returned nothing" fallback inside the respawn loop that appears structurally unreachable given the surrounding control flow. LinkedDataFragmentsServer.ts: covered the default no-op _log fallback.

Constructor validation, controller ordering, datasource error handling, logging setup, and run()'s full lifecycle (listen-when-ready, port override, SIGINT stop, forced second-SIGINT exit) — all driven through stubbed process.once/on, matching the pattern CliRunner-test.js already uses for process/signal handling, so nothing is ever registered on the real process listeners. Found a fourth pre-existing bug along the way (not fixed): access-log's own implementation treats a `null` third argument as an options object, since `typeof null === 'object'` — so the accesslogger this file builds throws every time it's actually invoked. Access logging is completely broken; the test documents that rather than working around it.

feature-memento, feature-qpf, and feature-summary Controller.ts is now at 100% branches. Small, previously-untested branches closed: pre-built ViewCollection reuse, an already-set parsedUrl/Vary header, a Forwarded header without proto, a double-invoked next()/done(), View's error-to-response-emit path, N3/RdfaDatasource's file-option fallback, DatasourceRouter's missing-parsedUrl fallback, SummaryController with no configured summaries directory, MementoHtmlViewExtension with no timegates configured, and QuadPatternFragmentsRdfView's no-pageUrl short-circuit.

…ller Datasource.ts: subject/object blank-node translation on both the query and result side, plus select() without an onError callback. (Several other flagged branches on this file turned out to be a coverage-tool sourcemap artifact, not real gaps — confirmed by checking raw lcov statement hit counts, which showed those exact lines executing thousands of times across the existing HDT/SPARQL integration tests.) QuadPatternFragmentsController.ts is now at 100% branches: _createPatternString's full term/graph matrix, the error-without-stack log path, and the previousPageUrl branch on page >= 2.

Closes the last real branch gap in Util.ts: no existing test constructed an error without a message, so the `message || ''` fallback was never exercised.

…r tests qejs's template resolver reads require.main.filename when resolving a template referenced via inherits(...). Vitest's workers have no classic CommonJS entry script, so require.main is undefined there and that access throws. The throw happens inside an unterminated Q promise chain, so Q silently swallows it instead of surfacing it, and response.emit('error', ...) throwing for the same reason (no listener attached) skips the response.end() call right after it, leaving the response hanging forever. Fix: a Vitest setupFiles script sets process.mainModule if unset, so require.main resolves the way qejs assumes. Un-skip the 3 describe blocks this was blocking, and stop forcing NotFoundHtmlView/NotFoundRdfView's require() onto the compiled .js output, matching how the rest of the suite requires views (extensionless, resolving through Vite to .ts source).

…r), Controller, HtmlView, SparqlDatasource, and N3ParserExtended; ignore coverage/

…iewExtension regressions from upstream refinement

Michiel-VandeVelde · 2026-08-21T12:18:03Z

@jitsedesmet ready for review :)

jitsedesmet

I sampled some files, and I found two things. By no means is this a complete review, but since I expect I'll have less to say beyond this, I would request these changes already.

(I also expect the line diff for these changes might be great and would unpin all my file-views)

jitsedesmet · 2026-08-21T12:41:14Z

 /*! @license MIT ©2015-2016 Ruben Verborgh, Ghent University - imec */

 const http = require('http');
+const sinon = require('sinon');


Do we still need sinon? Vitest has it's own spies I think?

jitsedesmet · 2026-08-21T12:47:55Z

+    it('should emit all triples in the SPARQL response', () => new Promise((done) => {
+      streamLength(result).then((length) => { expect(length).toBe(55); done(); });
+    }));


Suggested change

it('should emit all triples in the SPARQL response', () => new Promise((done) => {

streamLength(result).then((length) => { expect(length).toBe(55); done(); });

}));

it('should emit all triples in the SPARQL response', async () => {

const length = await streamLength(result);

expect(length).toBe(55);

});

In modern JS there is (mostly) no longer a need to handle promises directly.
I think, since you refactor the tests significantly throughout. Now might be a good time to refactor these things to be more modern also.

Having a good linter config will definitely help with this!
There are linter rules for this; see comunica's eslint config.

Michiel-VandeVelde added 17 commits August 21, 2026 11:51

Convert test suite from Mocha/Chai/Sinon to Vitest, keep .js

fcb3fe5

Splits the Mocha->Vitest and JS->TS conversions into two steps so each diff stays small and reviewable. This is step 1: syntax only, no types.

Raise test coverage on feature-memento's memento view (0% -> 100%)

5cb1e30

Add missing test coverage for core Util and UrlData

f3dd32f

Util.ts: 87% -> 100% statements. UrlData.ts: found a third pre-existing bug along the way (not fixed): options.assetsPath is always ignored, since baseURLPath + 'assets/' is never an empty string and short-circuits the || fallback.

Cover SparqlDatasource's remaining _encodeObject/_getPatternCount bra…

bfc0d46

…nches (96.6% -> 100%)

Exclude non-source files from coverage collection to stop spurious pa…

dddb598

…rse-error noise

Test the message-defaulting branch in createErrorType's ErrorType

c5c379f

Closes the last real branch gap in Util.ts: no existing test constructed an error without a message, so the `message || ''` fallback was never exercised.

Raise test coverage across CliRunner, LinkedDataFragmentsServer(Worke…

0bc7e15

…r), Controller, HtmlView, SparqlDatasource, and N3ParserExtended; ignore coverage/

Trim explanatory comments to essentials, mark known bugs as TODOs

168d194

Fix N3ParserExtended rename, document TimegateController/SummaryHtmlV…

f047fd0

…iewExtension regressions from upstream refinement

jitsedesmet self-requested a review August 21, 2026 12:21

jitsedesmet requested changes Aug 21, 2026

View reviewed changes

Navigation Menu

Uh oh!

Improve test coverage and change test suite to vitest - #220

Improve test coverage and change test suite to vitest#220
Michiel-VandeVelde wants to merge 17 commits into
LinkedDataFragments:masterfrom
Michiel-VandeVelde:improve-test-coverage-vitest-js

Michiel-VandeVelde commented Aug 21, 2026

Uh oh!

Michiel-VandeVelde commented Aug 21, 2026

Uh oh!

jitsedesmet left a comment •

edited

Loading

Uh oh!

jitsedesmet Aug 21, 2026

Uh oh!

jitsedesmet Aug 21, 2026

Uh oh!

rubensworks Aug 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Michiel-VandeVelde commented Aug 21, 2026

Uh oh!

Michiel-VandeVelde commented Aug 21, 2026

Uh oh!

jitsedesmet left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jitsedesmet Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

jitsedesmet Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

rubensworks Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jitsedesmet left a comment •

edited

Loading