Skip to content

Add unit tests for otsUtils and coverage evaluation report#1

Merged
UozumiMizuto merged 2 commits into
mainfrom
codex/evaluate-test-case-coverage
Apr 14, 2026
Merged

Add unit tests for otsUtils and coverage evaluation report#1
UozumiMizuto merged 2 commits into
mainfrom
codex/evaluate-test-case-coverage

Conversation

@UozumiMizuto

Copy link
Copy Markdown
Owner

Motivation

  • Improve test coverage for otsUtils utilities and document current coverage state for protocol and utils.
  • Surface untested branches (filesystem recursion, stamping/upgrading flows, and OpenTimestamps interaction) so they can be targeted by future tests.

Description

  • Add COVERAGE_EVALUATION.md summarizing scope checked, well-covered areas, uncovered behaviors, tooling status, and recommended next tests.
  • Add src/otsUtils.test.ts which provides a mocked in-memory filesystem and mocked window.OpenTimestamps to unit-test otsUtil behaviors.
  • Implement tests for getAllFilesRecursive filtering, generateRepoHash protocol equivalence, upgradeAll behaviors for missing and stale entries (rewrites .ots and clears pending registry), and stampHash / getInfo delegation to OpenTimestamps.
  • Use vitest test helpers and spies (vi) to assert interactions and side effects.

Testing

  • Ran the new unit tests with Vitest and the src/otsUtils.test.ts suite; tests completed and asserted expected behavior for all covered cases.
  • Attempted a coverage run with npx vitest run --config vitest.config.ts --coverage, but it failed due to a missing provider dependency: @vitest/coverage-v8 which prevented line/branch coverage output.
  • No other automated test failures were observed in the added test file execution.

Codex Task

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a test coverage evaluation document and a new unit test suite for otsUtils.ts. The tests utilize a custom mock filesystem to verify file recursion, repository hashing, and OpenTimestamps integration. Review feedback suggests enhancing test isolation by clearing global state in afterEach, expanding test coverage for time-based upgrade logic, and adopting Vitest's fake timers to manage internal timeouts.

Comment thread src/otsUtils.test.ts
Comment on lines +89 to +91
beforeEach(() => {
vi.restoreAllMocks();
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The tests modify the global window.OpenTimestamps object. To prevent side effects and ensure test isolation, you should reset this property after each test.

Suggested change
beforeEach(() => {
vi.restoreAllMocks();
});
afterEach(() => {
vi.restoreAllMocks();
delete (window as any).OpenTimestamps;
});

Comment thread src/otsUtils.test.ts
(window as any).OpenTimestamps = ots;

const fs = makeFs({
'/.tool/settings/ots/pending.json': JSON.stringify({ abc123: null }),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current test for upgradeAll uses a null value for the last attempt, which only covers the !lastAttempt branch. To fully verify the logic in otsUtils.ts, you should also include test cases for the 2-hour threshold (e.g., one entry that is too fresh to upgrade and one that is old enough).

Comment thread src/otsUtils.test.ts
@@ -0,0 +1,186 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The upgradeAll function in otsUtils.ts uses a setTimeout for a 30-second timeout during the upgrade process. In a testing environment, this can lead to hanging processes or flaky tests if the timer is not managed. It is recommended to use Vitest's fake timers to control time-dependent logic.

Suggested change
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

@UozumiMizuto UozumiMizuto merged commit 38886e4 into main Apr 14, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant