Added Node.js unit tests for core JavaScript utilities by xr843 · Pull Request #636 · buddhist-uni/buddhist-uni.github.io

xr843 · 2026-03-18T22:41:15Z

Summary

Added unit tests for assets/js/utils.js covering utils.unaccented, UpdateQueryString, locationOf, sortedInsert, and the Ranges class (interval set data structure)
Added unit tests for assets/js/buggytrack.js covering the cyrb53 hash function (determinism, uniqueness, edge cases)
Added unit tests for pure functions extracted from assets/js/search_index.js: categoryName, getPositions, and resultMatched
Updated package.json to add a "test" script: node --test test/**/*.test.js

Zero new dependencies — uses only Node.js built-in node:test and node:assert modules, plus node:vm to load the browser-global JS files into a sandbox.

40 tests across 9 suites, all passing.

This was discussed and agreed to with the maintainer @khinsen-bot (Khemarato Bhikkhu).

Test plan

All 40 tests pass locally with npm test
Verify tests pass in CI (Node 25 via actions/setup-node@v4)

Co-Authored-By: Claude Opus 4.6 (1M context) [email protected]

🤖 Generated with Claude Code

Added tests for utils.js (unaccented, UpdateQueryString, locationOf, sortedInsert, Ranges) and buggytrack.js (cyrb53) using Node's built-in test runner. Zero new dependencies. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

netlify · 2026-03-18T22:41:21Z

✅ Deploy Preview for obu ready!

Name	Link
🔨 Latest commit	`e5bd70e`
🔍 Latest deploy log	https://app.netlify.com/projects/obu/deploys/69bf6b55d60d9a0008675c08
😎 Deploy Preview	https://deploy-preview-636--obu.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

khemarato

Some tests! Excellent!

khemarato · 2026-03-18T23:34:19Z

package.json

  },
-  "scripts": {}
+  "scripts": {
+    "test": "node --test test/**/*.test.js"


Let's put these tests closer to the files they're testing in assets/js/

Also: make sure to exclude these js test files from the Jekyll build in _config

Fixed in the latest commit. Moved test files to assets/js/tests/ and added the exclusion pattern in _config.yml to keep them out of the Jekyll build.

assets/js/tests/buggytrack.test.js

khemarato · 2026-03-18T23:43:29Z

test/search-worker.test.js

+// Helper: bring vm-realm objects into the current realm for deepEqual
+function toLocal(obj) { return JSON.parse(JSON.stringify(obj)); }
+
+// search_index.js is a Jekyll/Liquid template, so we cannot load it


Let's maybe run these tests after the Jekyll build process for now, so you can grab the real output js? Or alternatively we can split search_index.js into two files (js code and document data) as a first PR before these tests. But I am not a fan of trying to build a custom parser inside a test 😅

Agreed — the custom parser is brittle and not ideal. I see two viable paths:

Split search_index.js into pure JS logic + Liquid data template — cleanest long-term, and tests can import the JS directly

Run tests post-build against the generated output — simpler setup but couples tests to the build pipeline

I'd lean toward option 1 since it also improves the code structure. Happy to submit that as a preliminary PR. Which approach do you prefer?

Sounds good! I'll submit a preliminary PR to split search_index.js into pure JS logic (search_functions.js) + Liquid data template first. Once that's merged, I'll rebase this PR to:

Remove the extractFunction workaround and import the split file directly

Add html-validate as a devDependency for HTML fragment validation

Add the "finds the section with the most matches" test case for getBlurbForResult

Validate HTML output from categoryName, addMatchHighlights, and getBlurbForResult

khemarato · 2026-03-18T23:59:08Z

test/search-worker.test.js

+
+describe('categoryName', () => {
+  it('returns correct HTML for known categories', () => {
+    assert.ok(categoryName('av').includes('Recording'));


These are over-specified and trivial tests. Instead of testing that each category has the right name, let's just make sure they are valid html fragments. We can leave the exact names to the function itself.

Good call. Fixed in the latest commit — now only validates that the return value contains a valid <i> icon element and is non-empty HTML, without asserting specific display names.

khemarato · 2026-03-19T00:03:46Z

test/search-worker.test.js

+    const result = { matchData: { metadata: {} } };
+    assert.equal(resultMatched(result, 'title'), false);
+  });
+});


Let's also add tests for addMatchHighlights and getBlurbForResult while we're here? (Nonblocking: can do in a follow-up PR if you like)

Added tests for both addMatchHighlights and getBlurbForResult in the latest commit.

…rage - Moved test files from test/ to assets/js/tests/ (closer to source) - Added assets/js/tests/ to _config.yml exclude list - Updated categoryName tests to validate HTML structure instead of specific names - Added tests for addMatchHighlights and getBlurbForResult - Added inline comments explaining why vm.createContext needs explicit globals Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

khemarato

Agreed. Let's go ahead and split search_index.js first so we can run these tests before the build.

assets/js/tests/search-worker.test.js

khemarato · 2026-03-19T01:27:27Z

assets/js/tests/search-worker.test.js

+    const content = 'a '.repeat(50) + 'MATCH' + ' b'.repeat(200);
+    const item = { title: 'Test', description: null, content: content };
+    const blurb = getBlurbForResult(result, item, [50]);
+    assert.ok(blurb.includes('MATCH') || blurb.includes('<strong>'));


assets/js/tests/search-worker.test.js

xr843 · 2026-03-19T14:05:07Z

Thanks for reviewing #638! Since you prefer keeping search_index.js as-is and testing manually, I'll update this PR to:

Remove the custom extractFunction parser for search_index.js tests
Keep the pure JS tests (buggytrack, search-functions, utils) that don't need the split
Add html-validate for HTML fragment validation as you suggested

Does that sound right, or would you prefer a different approach?

khemarato · 2026-03-19T14:11:40Z

Yep that sounds about right. You can also add tests for your new handleSearchMessage function.

…tests Address review feedback: - Remove extractFunction parser since search_functions.js now exists as a standalone file (after buddhist-uni#637 merged) - Load search_functions.js directly via vm sandbox - Add handleSearchMessage tests covering: stop word handling, +prefix logic, fallback search, filterquery filtering, and result structure validation Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

xr843 · 2026-03-19T23:15:38Z

Updated! Changes in the latest push:

Removed extractFunction parser — now that Split search_index.js into pure JS logic and Liquid data template #637 merged and search_functions.js exists as a standalone file, the test loads it directly via vm.runInContext()
Added handleSearchMessage tests (8 new test cases) covering:
- Result object structure validation
- Stop word detection (e.g. "the" is not prefixed with +)
- + prefix logic for required matching
- Preserving existing +/- prefixes
- Fallback to unprefixed query when required query returns nothing
- filterquery filtering
- Pass-through of q/filterquery/qt fields

All 57 tests pass (12 suites, 0 failures).

Also merged upstream main to resolve the out-of-date branch.

assets/js/tests/search-worker.test.js

Address review feedback from khemarato: add test cases covering all prefix combinations to verify existing +/- prefixes are preserved. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

khemarato · 2026-03-21T10:52:58Z

Ping: There are four unaddressed comments above, @xr843 No rush, just alerting you in case they dropped from your inbox. 😊

xr843 · 2026-03-21T11:29:55Z

@khemarato Done! Addressed all 4 comments:

Added html-validate for HTML fragment validation on addMatchHighlights and getBlurbForResult
Fixed the weak || assertion to check <strong>MATCH</strong> directly
Added "finds the section with the most matches" test case
Also fixed a wrong position offset in the test data

60 tests passing now. Let me know if anything else needs adjustment. Have a great day! 🙏

khemarato

Okay. Two more nits and then this should be good. 😊

assets/js/tests/search-worker.test.js

khemarato · 2026-03-22T00:47:13Z

assets/js/tests/search-worker.test.js

+    // Should pick the section with the most matches (MATCH, ME, INSTEAD) not the lone IGNORE
+    assert.ok(blurb.includes('MATCH'), 'Expected blurb to contain MATCH');
+    assert.ok(blurb.includes('INSTEAD'), 'Expected blurb to contain INSTEAD');
+    assert.ok(!blurb.includes('IGNORE'), 'Expected blurb to NOT contain IGNORE');


…order Address review nits: - Add assertValidHtml() for every category in the knownCategories loop - Flip assertion order in "finds the section with the most matches" to catch incorrect greedy implementations by checking the negative first Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

for (var i in positions) yields string keys ("0", "1", ...), causing j = i + 1 to produce "01" (string concatenation) instead of 1 (numeric addition). This made the inner while-loop skip positions[1] for i=0 and exit immediately for all other starting indices, so the algorithm always selected the first match's window regardless of where the densest cluster was. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

Place the lone IGNORE match before the cluster (MATCH, ME, INSTEAD) so that a greedy algorithm picking the first match would fail. Also add assertValidHtml() in the categoryName loop per review. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

khemarato

🐛 Anumodana

khemarato · 2026-03-22T06:07:21Z

assets/js/search_functions.js

    var best_i = -1;
    var best_n = 0;
-    for (var i in positions) {
+    for (var i = 0; i < positions.length; i++) {


Excellent! This is why I asked for the flipped test. I had a feeling there was a bug! 😁

Merge branch 'main' into feat/node-tests

767397e

khemarato reviewed Mar 19, 2026

View reviewed changes

khemarato requested changes Mar 19, 2026

View reviewed changes

This was referenced Mar 19, 2026

Split search_index.js into pure JS logic and Liquid data template #637

Merged

Split search_index.js into Liquid data and pure JS #638

Closed

xr843 and others added 2 commits March 20, 2026 07:13

Merge remote-tracking branch 'upstream/main' into feat/node-tests

13d2f57

khemarato reviewed Mar 20, 2026

View reviewed changes

assets/js/tests/search-worker.test.js Show resolved Hide resolved

test: add +-, --, -+ prefix combination tests for handleSearchMessage

347f68c

Address review feedback from khemarato: add test cases covering all prefix combinations to verify existing +/- prefixes are preserved. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

test: address review feedback — add html-validate and missing test cases

11747da

khemarato reviewed Mar 22, 2026

View reviewed changes

xr843 and others added 3 commits March 22, 2026 10:20

khemarato approved these changes Mar 22, 2026

View reviewed changes

khemarato merged commit 33b7f68 into buddhist-uni:main Mar 22, 2026
4 checks passed

xr843 deleted the feat/node-tests branch March 23, 2026 02:34

Conversation

xr843 commented Mar 18, 2026

Summary

Test plan

Uh oh!

netlify bot commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for obu ready!

Uh oh!

khemarato left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

khemarato left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

xr843 commented Mar 19, 2026

Uh oh!

khemarato commented Mar 19, 2026

Uh oh!

xr843 commented Mar 19, 2026

Uh oh!

Uh oh!

khemarato commented Mar 21, 2026

Uh oh!

xr843 commented Mar 21, 2026

Uh oh!

khemarato left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

khemarato left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

netlify bot commented Mar 18, 2026 •

edited

Loading