Skip to content

fix(tests): resolve unit test failures on development#2055

Merged
andrew-bierman merged 2 commits into
developmentfrom
fix/dev-unit-tests
Apr 11, 2026
Merged

fix(tests): resolve unit test failures on development#2055
andrew-bierman merged 2 commits into
developmentfrom
fix/dev-unit-tests

Conversation

@andrew-bierman
Copy link
Copy Markdown
Collaborator

Summary

Hotfix. Unit Tests CI on development is failing after recent feature PR merges. This PR restores green unit tests.

Root cause

All 312 Expo unit tests actually passed, but the Expo Unit Tests job runs vitest run --coverage with a 75% statement-coverage threshold. apps/expo/vitest.config.ts includes every .ts file under utils/, lib/utils/, and features/**/utils/** in the coverage report. After #1882 (Social feed) merged features/feed/utils/index.ts with no tests — alongside pre-existing untested files like features/pack-templates/utils/computePacktemplateWeight.ts, features/packs/utils/computeCategories.ts, and features/packs/utils/uploadImage.ts — the global statement coverage dropped to 74.32%, under the 75% floor, and v8 failed the job with:

ERROR: Coverage for statements (74.32%) does not meet global threshold (75%)

First failing run was commit dc4e8c9a (merge of #1882), last green was 6ce6d5b0.

Fix

Added pure-logic unit tests for the two easiest-to-cover new modules:

  • features/feed/utils/__tests__/feedUtils.test.ts — covers buildPostImageUrl, formatAuthorName (all name-presence branches, both Post and Comment), and the formatRelativeDate deprecated alias. clientEnvs and getRelativeTime are mocked.
  • features/pack-templates/utils/__tests__/computePacktemplateWeight.test.ts — covers empty templates, base gear only, consumable items (excluded from base, included in total), worn items (same), quantity multiplication, unit conversion to kg, and metadata preservation. The expo-app/features/packs/utils barrel is mocked down to the two pure conversion helpers so the test stays in a Node environment (the full barrel re-exports uploadImage.ts, which pulls expo-file-systemexpo-sqlite).

No production code changes. No .skip, no as casts, no @ts-ignore.

Result: coverage 74.32% → 79.87% statements, 330/330 tests passing.

Test plan

  • bun run test:expo passes locally (17 → 19 files, 312 → 330 tests)
  • bun run test:api:unit passes locally (18 files, 284 tests)
  • apps/expo bun run test:coverage exits 0 with 79.87% statements
  • CI Unit Tests job green

The Expo Unit Tests CI job on development was failing because the v8
statement-coverage threshold (75%) dropped to 74.32% after recent feature
merges. PR #1882 (social feed) and the pack-template weight helpers
landed with no unit tests, and vitest.config.ts includes every .ts under
`features/**/utils/**` in the coverage report.

This adds pure-logic unit tests for two easy-to-cover modules:

- features/feed/utils/index.ts: buildPostImageUrl, formatAuthorName,
  and formatRelativeDate. clientEnvs and getRelativeTime are mocked.
- features/pack-templates/utils/computePacktemplateWeight.ts: covers
  empty templates, base gear, consumables, worn items, quantity, and
  unit conversion. The expo-app/features/packs/utils barrel is mocked to
  the two pure conversion helpers so the test does not pull in
  uploadImage.ts (which imports expo-file-system -> expo-sqlite).

Coverage goes from 74.32% -> 79.87% statements; 330/330 tests pass.
No production code changes.
Copilot AI review requested due to automatic review settings April 11, 2026 13:00
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 11, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 924311c1-e199-4b34-8a7b-0fa5591952dc

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/dev-unit-tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

The Unit Tests workflow path filter only listed
`apps/expo/features/packs/utils/**`, so changes under other feature
utils directories (e.g. `features/feed/utils/**`,
`features/pack-templates/utils/**`) did not trigger the job even though
they are included in `apps/expo/vitest.config.ts` coverage report.

Broaden the filter to `apps/expo/features/**/utils/**` to match the
vitest `include` pattern, and also trigger on changes to the workflow
itself so edits here are verified by a run.
@github-actions github-actions Bot added the ci/cd label Apr 11, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Coverage Report for Expo Unit Tests Coverage (./apps/expo)

Status Category Percentage Covered / Total
🔵 Lines 79.87% 532 / 666
🔵 Statements 79.87% (🎯 75%) 532 / 666
🔵 Functions 94.64% 53 / 56
🔵 Branches 92.72% 204 / 220
File CoverageNo changed files found.
Generated in workflow #81 for commit 0a36a08 by the Vitest Coverage Report Action

@github-actions
Copy link
Copy Markdown
Contributor

Coverage Report for API Unit Tests Coverage (./packages/api)

Status Category Percentage Covered / Total
🔵 Lines 89.66% 902 / 1006
🔵 Statements 89.66% (🎯 80%) 902 / 1006
🔵 Functions 97.95% 48 / 49
🔵 Branches 89.74% 280 / 312
File CoverageNo changed files found.
Generated in workflow #81 for commit 0a36a08 by the Vitest Coverage Report Action

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Restores green Expo unit-test CI by adding targeted unit tests to raise global statement coverage above the configured threshold, without changing production code.

Changes:

  • Added unit tests for feed/utils helpers (buildPostImageUrl, formatAuthorName, formatRelativeDate) with deterministic mocks.
  • Added unit tests for computePackTemplateWeights, including unit conversion and base-vs-total weight rules, while mocking the packs utils barrel to keep tests Node-safe.

Reviewed changes

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

File Description
apps/expo/features/pack-templates/utils/tests/computePacktemplateWeight.test.ts Adds coverage for pack-template weight computation logic, including conversions and item classification rules.
apps/expo/features/feed/utils/tests/feedUtils.test.ts Adds coverage for feed utility helpers with mocks for env and time formatting.

Comment on lines +53 to +55
it('does not double-encode or trim the image key', () => {
expect(buildPostImageUrl('folder/sub folder/image (1).png')).toBe(
'https://cdn.example.com/folder/sub folder/image (1).png',
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

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

This test locks in behavior that produces URLs containing spaces/parentheses unencoded, which is not a valid/robust URL form in many clients. If buildPostImageUrl is intended to return a safe URL, update the expectation to percent-encode reserved characters (e.g., spaces as %20), or alternatively change the test input to an already-encoded key and assert it is not double-encoded (so the test still covers the intended contract reliably).

Suggested change
it('does not double-encode or trim the image key', () => {
expect(buildPostImageUrl('folder/sub folder/image (1).png')).toBe(
'https://cdn.example.com/folder/sub folder/image (1).png',
it('does not double-encode or trim an already-encoded image key', () => {
expect(buildPostImageUrl('folder/sub%20folder/image%20%281%29.png')).toBe(
'https://cdn.example.com/folder/sub%20folder/image%20%281%29.png',

Copilot uses AI. Check for mistakes.
Comment on lines +3 to +15

// The SUT imports convertFromGrams/convertToGrams from the packs/utils barrel,
// which also re-exports uploadImage (expo-file-system) and computeCategories
// (expo-app/features/auth/store). Replace the barrel with the real pure
// conversion helpers so the test stays in a Node environment.
vi.mock('expo-app/features/packs/utils', async () => {
const fromGrams = await import('expo-app/features/packs/utils/convertFromGrams');
const toGrams = await import('expo-app/features/packs/utils/convertToGrams');
return {
convertFromGrams: fromGrams.convertFromGrams,
convertToGrams: toGrams.convertToGrams,
};
});
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

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

Mocking the entire expo-app/features/packs/utils barrel by reaching into internal module paths makes these tests sensitive to refactors (file moves/renames) in the packs utils area. A more maintainable approach is to make the production module under test import the pure conversion helpers directly (not via the barrel), which removes the need for this barrel mock and keeps the unit pure across environments. If production changes are out of scope here, consider at least centralizing this mock into a reusable mock module to avoid repeating fragile path logic in future tests.

Suggested change
// The SUT imports convertFromGrams/convertToGrams from the packs/utils barrel,
// which also re-exports uploadImage (expo-file-system) and computeCategories
// (expo-app/features/auth/store). Replace the barrel with the real pure
// conversion helpers so the test stays in a Node environment.
vi.mock('expo-app/features/packs/utils', async () => {
const fromGrams = await import('expo-app/features/packs/utils/convertFromGrams');
const toGrams = await import('expo-app/features/packs/utils/convertToGrams');
return {
convertFromGrams: fromGrams.convertFromGrams,
convertToGrams: toGrams.convertToGrams,
};
});
import { convertFromGrams } from 'expo-app/features/packs/utils/convertFromGrams';
import { convertToGrams } from 'expo-app/features/packs/utils/convertToGrams';
// The SUT imports convertFromGrams/convertToGrams from the packs/utils barrel,
// which also re-exports uploadImage (expo-file-system) and computeCategories
// (expo-app/features/auth/store). Replace the barrel with the real pure
// conversion helpers so the test stays in a Node environment.
vi.mock('expo-app/features/packs/utils', () => ({
convertFromGrams,
convertToGrams,
}));

Copilot uses AI. Check for mistakes.
@andrew-bierman andrew-bierman merged commit d2e0bf5 into development Apr 11, 2026
5 of 9 checks passed
@andrew-bierman andrew-bierman deleted the fix/dev-unit-tests branch April 15, 2026 02:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants