test(utils): add tests for misc, MemCache, and DiskCache by marcoscaceres · Pull Request #502 · speced/respec-web-services

marcoscaceres · 2026-04-25T10:36:12Z

Summary

19 specs for env(), seconds(), ms(), HTTPError
24 specs for MemCache (TTL, stale reads, getOr, invalidate)
11 specs for DiskCache (persistence, path traversal guard)

Copilot

Pull request overview

Adds comprehensive Jasmine coverage for utility helpers and both cache implementations, strengthening regression protection around TTL behavior, persistence, and input validation.

Changes:

Add unit tests for env(), seconds(), ms(), and HTTPError.
Add behavioral tests for MemCache (TTL expiry, stale reads, getOr(), invalidate(), etc.).
Add integration-style tests for DiskCache (persistence across instances, TTL expiry/invalidation, and path traversal key rejection).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
tests/utils/misc.test.js	New specs for misc utility helpers and `HTTPError`.
tests/utils/mem-cache.test.js	New specs validating `MemCache` correctness across key behaviors.
tests/utils/disk-cache.test.js	New specs validating `DiskCache` persistence, TTL behavior, and key/path safety checks.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-25T13:11:50Z

+      const cache = new DiskCache({ ttl: 1, path: "test-cache" });
+      await cache.set("key", "value");
+
+      // Wait briefly to ensure TTL expires
+      await new Promise(resolve => setTimeout(resolve, 10));
+      expect(await cache.get("key")).toBeUndefined();
+    });


These TTL tests use real setTimeout sleeps to force expiry, which slows the suite and can be timing-sensitive. Consider stubbing Date.now() (or using a fake clock) to advance time deterministically and avoid waiting in tests.

Valid suggestion. The DiskCache TTL tests use real setTimeout(10ms) which is timing-dependent. Stubbing Date.now() would be more deterministic. However, with 10ms waits the flakiness risk is very low in practice, and DiskCache reads file mtimes from disk which adds complexity to mocking. Keeping as-is for now — can refactor to fake clocks as a follow-up if flakiness is observed.

Copilot · 2026-04-25T13:11:50Z

+      const cache = new DiskCache({ ttl: 1, path: "test-cache" });
+      await cache.set("a", 1);
+      await cache.set("b", 2);
+
+      await new Promise(resolve => setTimeout(resolve, 10));
+      await cache.invalidate();
+
+      expect(await cache.get("a", true)).toBeUndefined();
+      expect(await cache.get("b", true)).toBeUndefined();
+    });


Same as above: relying on setTimeout(..., 10) to age entries makes the test slower and more timing-dependent than necessary. Stubbing Date.now()/using a fake clock would let you test invalidate() behavior without sleeping.

Valid suggestion. The DiskCache TTL tests use real setTimeout(10ms) which is timing-dependent. Stubbing Date.now() would be more deterministic. However, with 10ms waits the flakiness risk is very low in practice, and DiskCache reads file mtimes from disk which adds complexity to mocking. Keeping as-is for now — can refactor to fake clocks as a follow-up if flakiness is observed.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-25T13:16:38Z

+
+      await new Promise(resolve => setTimeout(resolve, 10));
+      await cache.invalidate();


Similar to the TTL expiry tests, this setTimeout-based wait can introduce timing flakiness. Prefer stubbing Date.now() to move time forward, which keeps the test deterministic and avoids adding real delays to CI runs.

Valid suggestion. The DiskCache TTL tests use real setTimeout(10ms) which is timing-dependent. Stubbing Date.now() would be more deterministic. However, with 10ms waits the flakiness risk is very low in practice, and DiskCache reads file mtimes from disk which adds complexity to mocking. Keeping as-is for now — can refactor to fake clocks as a follow-up if flakiness is observed.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Adds 54 new specs covering the three core utility modules: - misc: env(), seconds(), ms(), HTTPError (19 specs) - MemCache: set/get, TTL expiry, has, getOr, delete, clear, expires, invalidate (24 specs) - DiskCache: set/get round-trip, TTL expiry, persistence across instances, invalidate, path traversal prevention (11 specs)

…shot env() throws raw strings, not Error instances, so toThrow() can't reliably match messages. Switch to try/catch with exact string assertion. Reset savedEnv in beforeEach so each test tracks only its own keys.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

marcoscaceres · 2026-04-27T08:15:46Z

@copilot can you address any unaddressed concerns? O

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…versal prefix bypass - Remove orphaned describe("TTL expiry") + it() block (lines 65-75) that was never closed, causing "Unexpected end of input" syntax error at test load time - Fix path-traversal prefix-bypass in DiskCache.keyToFilePath(): use `baseDir + path.sep` instead of bare `baseDir` so a key like `../test-cache-evil/pwn` cannot escape the cache directory - Add test for the prefix-bypass scenario Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/505fff65-5d3f-41b9-8488-88bfebd453bb Co-authored-by: marcoscaceres <870154+marcoscaceres@users.noreply.github.com>

Copilot · 2026-04-27T08:21:22Z

@copilot can you address any unaddressed concerns? O

Addressed the remaining unresolved concerns in commit 7dbbe88:

Syntax error in disk-cache.test.js — Removed the orphaned describe("TTL expiry") + it() block (lines 65-75) that was never closed, which caused an "Unexpected end of input" error and prevented the test suite from loading at all.
Path-traversal prefix-bypass vulnerability — Fixed DiskCache.keyToFilePath() to use result.startsWith(baseDir + path.sep) instead of bare result.startsWith(baseDir). Without the separator, a key like ../test-cache-evil/pwn resolves to /tmp/test-cache-evil/pwn.json which falsely passes the old check (since "test-cache-evil" starts with "test-cache").
Added prefix-bypass test — New spec "rejects prefix-bypass traversal (../test-cache-evil)" covers the exact scenario described in the review.

All 55 specs pass.

marcoscaceres · 2026-04-27T08:53:09Z

@copilot can you review this and fix any issues that come up?

- Set `tempDir = undefined` at the top of `beforeEach` so that a `mkdtemp` failure on a later run cannot accidentally reuse the previous test's directory in `afterEach` - Wrap env-var restoration in `try { } finally { rm... }` so the temp directory is always cleaned up even if the env restoration step throws Agent-Logs-Url: https://github.com/speced/respec-web-services/sessions/3c3383f4-458d-4aaf-8fc6-091cdefd4390 Co-authored-by: marcoscaceres <870154+marcoscaceres@users.noreply.github.com>

marcoscaceres · 2026-04-29T03:10:34Z

@copilot Can you confirm all your feedback has been addressed? Please resolve the threads that have been addressed and fix anything outstanding.

Copilot · 2026-04-29T03:12:51Z

@copilot Can you confirm all your feedback has been addressed? Please resolve the threads that have been addressed and fix anything outstanding.

All feedback has been addressed. Here's a summary of each open thread:

setTimeout vs fake clock (disk-cache threads on lines 66–72, 69–71, 75–79, 85–94, 88–90): The current code already uses spyOn(Date, "now").and.callFake(() => now) in a beforeEach — no real sleeps anywhere in TTL tests.
env var isolation (misc.test.js:18–44, 18–22): savedEnv = {} is reset in beforeEach and restored in afterEach; each test saves its own keys before mutating.
try/finally + tempDir guard (disk-cache.test.js:17–29): Fixed in 4a0bcd6 — tempDir = undefined at top of beforeEach, afterEach uses try/finally so cleanup always runs.
prefix-bypass path traversal (disk-cache.test.js:111–145): Fixed in 7dbbe88 — implementation uses startsWith(baseDir + path.sep) and a dedicated test covers the ../test-cache-evil bypass case.

All 55 specs pass. I can't resolve GitHub threads via these tools, but every concern in the open threads is addressed in the code.

marcoscaceres · 2026-05-04T13:13:01Z

@copilot can you resolve the merge conflicts?

# Conflicts: # tests/utils/misc.test.js

Copilot · 2026-05-04T13:17:36Z

@copilot can you resolve the merge conflicts?

Resolved in commit b3f97d1. The only conflict was in tests/utils/misc.test.js — merged our env(), seconds(), ms(), and HTTPError tests with main's new getErrnoCode tests, and updated the import to include getErrnoCode. utils/disk-cache.ts auto-merged cleanly (main switched it to use getErrnoCode() instead of error.code). All 63 specs pass.

marcoscaceres requested a review from Copilot April 25, 2026 12:25

Copilot started reviewing on behalf of marcoscaceres April 25, 2026 12:26 View session

Copilot AI reviewed Apr 25, 2026

View reviewed changes

Comment thread tests/utils/disk-cache.test.js Outdated

marcoscaceres force-pushed the test/core-utils branch from 6e23fc2 to 1b45559 Compare April 25, 2026 12:37

marcoscaceres requested a review from Copilot April 25, 2026 13:07

Copilot started reviewing on behalf of marcoscaceres April 25, 2026 13:08 View session

Copilot AI reviewed Apr 25, 2026

View reviewed changes

marcoscaceres requested a review from Copilot April 25, 2026 13:13

Copilot started reviewing on behalf of marcoscaceres April 25, 2026 13:13 View session

Copilot AI reviewed Apr 25, 2026

View reviewed changes

marcoscaceres requested a review from Copilot April 25, 2026 13:38

Copilot started reviewing on behalf of marcoscaceres April 25, 2026 13:38 View session

Copilot AI reviewed Apr 25, 2026

View reviewed changes

Comment thread tests/utils/misc.test.js

marcoscaceres force-pushed the test/core-utils branch from 1b45559 to 94a9a01 Compare April 26, 2026 00:17

marcoscaceres requested a review from Copilot April 26, 2026 01:20

Copilot started reviewing on behalf of marcoscaceres April 26, 2026 01:20 View session

Copilot AI reviewed Apr 26, 2026

View reviewed changes

Comment thread tests/utils/disk-cache.test.js Outdated

Comment thread tests/utils/disk-cache.test.js Outdated

Comment thread tests/utils/disk-cache.test.js

marcoscaceres requested a review from Copilot April 26, 2026 03:38

Copilot started reviewing on behalf of marcoscaceres April 26, 2026 03:39 View session

Copilot AI reviewed Apr 26, 2026

View reviewed changes

Comment thread tests/utils/disk-cache.test.js Outdated

Comment thread tests/utils/misc.test.js Outdated

Comment thread tests/utils/misc.test.js Outdated

marcoscaceres requested a review from Copilot April 26, 2026 03:59

Copilot started reviewing on behalf of marcoscaceres April 26, 2026 03:59 View session

Copilot AI reviewed Apr 26, 2026

View reviewed changes

Comment thread tests/utils/disk-cache.test.js

Comment thread tests/utils/misc.test.js

marcoscaceres requested a review from Copilot April 26, 2026 04:33

Copilot started reviewing on behalf of marcoscaceres April 26, 2026 04:34 View session

Copilot AI reviewed Apr 26, 2026

View reviewed changes

Comment thread tests/utils/disk-cache.test.js Outdated

Comment thread tests/utils/misc.test.js

marcoscaceres requested a review from Copilot April 27, 2026 00:11

Copilot started reviewing on behalf of marcoscaceres April 27, 2026 00:12 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

Comment thread tests/utils/misc.test.js Outdated

marcoscaceres requested a review from Copilot April 27, 2026 02:16

Copilot started reviewing on behalf of marcoscaceres April 27, 2026 02:17 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

marcoscaceres and others added 2 commits April 27, 2026 18:13

Apply suggestion from @Copilot

8620d21

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Apply suggestions from code review

d71b925

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot started work on behalf of marcoscaceres April 27, 2026 08:16 View session

marcoscaceres and others added 2 commits April 27, 2026 18:16

Apply suggestion from @Copilot

6c2b1b9

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot finished work on behalf of marcoscaceres April 27, 2026 08:23

Copilot started work on behalf of marcoscaceres April 27, 2026 08:54 View session

Copilot finished work on behalf of marcoscaceres April 27, 2026 09:02

Copilot started work on behalf of marcoscaceres April 29, 2026 03:10 View session

Copilot finished work on behalf of marcoscaceres April 29, 2026 03:13

marcoscaceres marked this pull request as ready for review April 29, 2026 03:14

marcoscaceres requested a review from sidvishnoi April 29, 2026 13:40

sidvishnoi approved these changes May 4, 2026

View reviewed changes

Copilot started work on behalf of marcoscaceres May 4, 2026 13:14 View session

Merge remote-tracking branch 'origin/main' into test/core-utils

b3f97d1

# Conflicts: # tests/utils/misc.test.js

Copilot finished work on behalf of marcoscaceres May 4, 2026 13:18

marcoscaceres merged commit 3abb21d into main May 4, 2026
3 checks passed

marcoscaceres deleted the test/core-utils branch May 4, 2026 13:23


		await new Promise(resolve => setTimeout(resolve, 10));
		await cache.invalidate();

Uh oh!

Conversation

marcoscaceres commented Apr 25, 2026

Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

marcoscaceres Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

marcoscaceres Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

marcoscaceres Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!