Skip to content

test(web): cover Smartling review pull#1277

Merged
cungminh2710 merged 3 commits into
mainfrom
cursor/missing-test-coverage-441b
Jul 6, 2026
Merged

test(web): cover Smartling review pull#1277
cungminh2710 merged 3 commits into
mainfrom
cursor/missing-test-coverage-441b

Conversation

@cursor

@cursor cursor Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

What changed

Added focused Smartling review-pull coverage for recently merged provider code.

Risky behavior now covered:

  • Smartling review pulls expand job source files into source-string hashcodes before requesting issues.
  • Smartling issue responses are normalized into provider review threads with state, issue type, item metadata, locale, comments, and provider URL.
  • Duplicate Smartling issue UIDs are deduplicated in the final review report.
  • Smartling 401 responses while loading review scope map to smartling_auth_invalid.

Test files added/updated:

  • apps/hyperlocalise-web/src/lib/providers/adapters/smartling/smartling-review-puller.test.ts

Why these tests materially reduce regression risk:

  • Review pull feeds QA/review reporting for provider-backed jobs, so bad normalization or missing job-file hashcodes could hide real Smartling review issues from users.
  • The test drives the real smartlingTmsProvider.pullReview() method through mocked Smartling API responses, covering API request shape and provider report output without external services.

How to test

  • vp test src/lib/providers/adapters/smartling/smartling-review-puller.test.ts
  • vp test
  • vp check --fix

Checklist

  • I ran relevant checks locally (make fmt, make lint, make test)
  • I updated docs, comments, or examples when needed
  • This PR is scoped and ready for review
Open in Web View Automation 

cursoragent and others added 2 commits July 6, 2026 21:05
Co-authored-by: Minh Cung <cungminh2710@users.noreply.github.com>
Co-authored-by: Minh Cung <cungminh2710@users.noreply.github.com>
@vercel

vercel Bot commented Jul 6, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
hyperlocalise Ignored Ignored Jul 6, 2026 10:47pm

Request Review

@cungminh2710
cungminh2710 marked this pull request as ready for review July 6, 2026 22:10
@cursor

cursor Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@greptile-apps

greptile-apps Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a new integration-style test file for smartlingTmsProvider.pullReview(), covering the core behaviors of the Smartling review-pull flow using a vi.fn() fetch mock. No production code is changed.

  • The happy-path test exercises job-file hashcode expansion, source-string fetching, issue normalization, and duplicate-issue deduplication, all against mocked Smartling API responses.
  • A second test verifies that 401 responses from the project-details endpoint propagate as smartling_auth_invalid rejections.

Confidence Score: 5/5

Test-only change with no production code modifications; safe to merge.

The PR adds one test file covering pullReview(). The mock routing, deduplication logic, and report-shape assertions all correctly reflect the implementation. No production code is touched.

No files require special attention.

Important Files Changed

Filename Overview
apps/hyperlocalise-web/src/lib/providers/adapters/smartling/smartling-review-puller.test.ts New test file covering pullReview(). Mock routing, assertions, and deduplication logic are all correct. One minor ordering fragility in the threads assertion.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Test
    participant pullReview
    participant SmartlingApi

    Test->>pullReview: pullReview(scope, fetchFn)
    pullReview->>SmartlingApi: POST /authenticate
    SmartlingApi-->>pullReview: "{ accessToken }"
    pullReview->>SmartlingApi: "GET /projects-api/v2/projects/{projectId}"
    SmartlingApi-->>pullReview: "{ accountUid, ... }"
    pullReview->>SmartlingApi: "GET /jobs-api/v3/projects/{projectId}/jobs/{jobId}/files"
    SmartlingApi-->>pullReview: "{ items: [{ fileUri }] }"
    pullReview->>SmartlingApi: "GET /strings-api/v2/projects/{projectId}/source-strings?fileUri=messages.json"
    SmartlingApi-->>pullReview: "{ items: [{ hashcode }] }"
    Note over pullReview: Merge content unit hashcodes + job-file hashcodes into Set
    pullReview->>SmartlingApi: POST /issues/list
    SmartlingApi-->>pullReview: "{ items: [issue-file-1, issue-content-1, issue-file-1 dup] }"
    Note over pullReview: Deduplicate by threadId via Map
    pullReview-->>Test: "ProviderReviewReport { threads: [2 unique], summary }"
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Test
    participant pullReview
    participant SmartlingApi

    Test->>pullReview: pullReview(scope, fetchFn)
    pullReview->>SmartlingApi: POST /authenticate
    SmartlingApi-->>pullReview: "{ accessToken }"
    pullReview->>SmartlingApi: "GET /projects-api/v2/projects/{projectId}"
    SmartlingApi-->>pullReview: "{ accountUid, ... }"
    pullReview->>SmartlingApi: "GET /jobs-api/v3/projects/{projectId}/jobs/{jobId}/files"
    SmartlingApi-->>pullReview: "{ items: [{ fileUri }] }"
    pullReview->>SmartlingApi: "GET /strings-api/v2/projects/{projectId}/source-strings?fileUri=messages.json"
    SmartlingApi-->>pullReview: "{ items: [{ hashcode }] }"
    Note over pullReview: Merge content unit hashcodes + job-file hashcodes into Set
    pullReview->>SmartlingApi: POST /issues/list
    SmartlingApi-->>pullReview: "{ items: [issue-file-1, issue-content-1, issue-file-1 dup] }"
    Note over pullReview: Deduplicate by threadId via Map
    pullReview-->>Test: "ProviderReviewReport { threads: [2 unique], summary }"
Loading

Reviews (2): Last reviewed commit: "test(web): harden Smartling review-pull ..." | Re-trigger Greptile

@cungminh2710

Copy link
Copy Markdown
Contributor

@cursoragent Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.


Issue 1 of 3

apps/hyperlocalise-web/src/lib/providers/adapters/smartling/smartling-review-puller.test.ts:49-51
An expect() call inside a vi.fn() mock handler is fragile: if the assertion fails, the error propagates as a rejection from fetchFn, causing pullReview to throw an unexpected network-like error rather than surfacing a clear Vitest assertion message. Capture the value and assert it outside the mock instead.

      if (path.includes("/strings-api/v2/projects/proj-1/source-strings") && method === "GET") {
        const capturedFileUri = new URL(path).searchParams.get("fileUri");
        return smartlingResponse({

Issue 2 of 3

apps/hyperlocalise-web/src/lib/providers/adapters/smartling/smartling-review-puller.test.ts:115
The issueRequestHashcodes assertion uses toEqual with a fixed order, coupling the test to the Set insertion sequence inside pullReview (content units first, then job-file strings). If the implementation ever reorders those two passes, this assertion fails even though the API call is still correct. Using expect.arrayContaining and a single-element outer array makes the intent clearer and the test more robust.

    expect(issueRequestHashcodes).toHaveLength(1);
    expect(issueRequestHashcodes[0]).toEqual(
      expect.arrayContaining(["hash-from-content", "hash-from-job-file"]),
    );
    expect(issueRequestHashcodes[0]).toHaveLength(2);

Issue 3 of 3

apps/hyperlocalise-web/src/lib/providers/adapters/smartling/smartling-review-puller.test.ts:89-96
as never bypasses all type checking on the scope object

The entire argument object is cast with as never, which silences every TypeScript error on the literal — including any required fields that TmsProviderPullReviewScope may define but that the test doesn't supply. A silent contract mismatch here would not be caught at compile time. Prefer casting only the parts that genuinely need it (e.g. fetchFn: fetchFn as typeof fetch) and satisfying the rest of the scope type explicitly, or using a typed partial helper.

@cursor

cursor Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

All three Greptile review issues are fixed in smartling-review-puller.test.ts.

Issue 1 — expect() inside mock handler

Captured fileUri in a capturedFileUris array during the mock call, then asserted outside after pullReview completes. Failures now surface as clear Vitest assertion errors instead of network-like rejections.

Issue 2 — Order-sensitive hashcode assertion

Replaced the fixed-order toEqual with:

  • toHaveLength(1) on the outer array
  • expect.arrayContaining([...]) for the hashcodes
  • toHaveLength(2) to confirm no extras

The test now validates the set of hashcodes without depending on insertion order.

Issue 3 — as never on entire scope

Removed the outer as never cast and supplied the required scope fields explicitly (organizationId, projectId, etc.). Narrow casts remain only where needed: credential and project as as never, and fetchFn as typeof fetch.

Both tests pass, and vp check --fix is clean. Changes are pushed to cursor/missing-test-coverage-441b.

Open in Web Open in Cursor 

Co-authored-by: Minh Cung <cungminh2710@users.noreply.github.com>
@cungminh2710
cungminh2710 merged commit e4dd93e into main Jul 6, 2026
9 checks passed
@cungminh2710
cungminh2710 deleted the cursor/missing-test-coverage-441b branch July 6, 2026 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants