Skip to content

refactor(test): reuse the Zalo mock-call assertion helper - #139294

Merged
steipete merged 1 commit into
mainfrom
codex/test-zalo-first-fetch-assertion
Sep 5, 2026
Merged

refactor(test): reuse the Zalo mock-call assertion helper#139294
steipete merged 1 commit into
mainfrom
codex/test-zalo-first-fetch-assertion

Conversation

@steipete

@steipete steipete commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

The Zalo API tests carry a local adapter for requiring the first mock fetch call, duplicating the existing SDK assertion helper. This maintainer-requested cleanup consolidates that assertion without removing coverage.

Why This Change Was Made

Use expectDefined(fetcher.mock.calls[0], label) at the five call sites and delete the local adapter. The generic SDK helper preserves the typed argument tuple and fails when the call is missing. Request-init and abort-signal guards remain explicit at their callers.

User Impact

No user-visible behavior changes. Production code is unchanged; tests are +6/-13 lines. All 20 API cases and 35 expectations remain, including request methods, timeouts, body handling, photo URL validation, SSRF rejection and cleanup.

Evidence

  • Complete API and loopback probe suites passed before and after: 24/24 each, with identical ordered case inventories per file.
  • The normal changed-file gate passed all 19 selected checks, including the plugin test type graph, all three dead-export scans and lint.
  • Two-pass managed Codex review found no actionable P0-P2 issues; source and index remained unchanged.

These local checks used base 3d129595202d65d60a363dec093f20861fb2bcf3. Current main independently removes fake timer handles in three of these tests; that change must remain intact at landing and is not included in this cleanup's LOC count. Fresh hosted CI must validate the merged candidate before landing. No live external Zalo service behavior is claimed or changed.

@clawsweeper

clawsweeper Bot commented Sep 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@openclaw-barnacle openclaw-barnacle Bot added channel: zalo Channel integration: zalo size: XS maintainer Maintainer-authored PR labels Sep 5, 2026
@clawsweeper clawsweeper Bot added P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 5, 2026
@clawsweeper

clawsweeper Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed September 5, 2026, 2:54 PM ET / 18:54 UTC.

ClawSweeper review

What this changes

The PR replaces Zalo tests’ local mock-call guard with the existing shared assertion helper, preserving coverage without changing production behavior.

Merge readiness

✅ Ready for maintainer review

No actionable findings. This useful, maintainer-requested cleanup remains absent from current main and preserves the existing assertions.

Priority: P3
Reviewed head: 5c4c9c9569560bc9f3e0777ce26d968a0ff98153

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, behavior-preserving cleanup with reported targeted validation and no actionable findings.
Proof confidence 🌊 off-meta tidepool Not applicable: The supplied MEMBER association exempts this maintainer-authored PR from external-contributor proof requirements; only test assertions change, with before/after suite results reported in the body.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The supplied MEMBER association exempts this maintainer-authored PR from external-contributor proof requirements; only test assertions change, with before/after suite results reported in the body.
Evidence reviewed 8 items Verified introduced change: The pinned introduction changes five mock-call reads and removes their local adapter; request-init guards, abort assertions, and test cases remain intact.
Shared helper contract: The existing generic helper returns the original typed value and throws for null or undefined. Mock-call entries are tuples or absent, so the replacement preserves the relevant failure condition. The helper has no imports or runtime side effects.
Existing supported reuse: An existing ElevenLabs test uses the same SDK import and first-fetch-call assertion pattern. The subpath is registered in package.json and scripts/lib/plugin-sdk-entrypoints.json; this PR adds no SDK API.
Findings None None.
Security None None.

How this fits together

The Zalo plugin translates messaging operations into HTTP requests. Its API tests inspect recorded fetch calls to check request construction, deadlines, and rejection behavior.

flowchart LR
  A[Test scenarios] --> B[Zalo API client]
  B --> C[Recorded fetch calls]
  C --> D[Shared presence assertion]
  D --> E[Request and abort assertions]
  E --> F[Test result]
Loading

Before merge

None.

Agent review details

Security

None.

PR surface

Tests -7. Total -7 across 1 file.

View PR surface stats
Area Files Added Removed Net
Source 0 0 0 0
Tests 1 6 13 -7
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 1 6 13 -7

Review metrics

Metric Value Why it matters
Production versus test LOC Production 0; tests net -7 (+6/-13) The reduction removes duplicated test support without adding a production seam or deleting behavioral coverage.

Technical review

Best possible solution:

Use the existing shared assertion while retaining request-specific guards, all behavioral coverage, and main’s independent timer cleanup.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this refactor reports duplicated test support, not a runtime bug; source inspection confirms the duplication.

Is this the best way to solve the issue?

Yes. Direct reuse preserves tuple typing and missing-call failures without another adapter; keeping the local helper would retain unnecessary duplication.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against d4d766d39fb2.

Labels

Label changes:

  • add P3: This is bounded test-maintenance cleanup with no user-visible behavior change.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The supplied MEMBER association exempts this maintainer-authored PR from external-contributor proof requirements; only test assertions change, with before/after suite results reported in the body.

Label justifications:

  • P3: This is bounded test-maintenance cleanup with no user-visible behavior change.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The supplied MEMBER association exempts this maintainer-authored PR from external-contributor proof requirements; only test assertions change, with before/after suite results reported in the body.

Evidence

What I checked:

  • Verified introduced change: The pinned introduction changes five mock-call reads and removes their local adapter; request-init guards, abort assertions, and test cases remain intact. (extensions/zalo/src/api.test.ts:80, 5c4c9c956956)
  • Shared helper contract: The existing generic helper returns the original typed value and throws for null or undefined. Mock-call entries are tuples or absent, so the replacement preserves the relevant failure condition. The helper has no imports or runtime side effects. (src/plugin-sdk/expect-runtime.ts:2, d4d766d39fb2)
  • Existing supported reuse: An existing ElevenLabs test uses the same SDK import and first-fetch-call assertion pattern. The subpath is registered in package.json and scripts/lib/plugin-sdk-entrypoints.json; this PR adds no SDK API. (extensions/elevenlabs/media-understanding-provider.test.ts:49, 5c4c9c956956)
  • Production boundary inspected: Read the complete API implementation and API tests, plus probe implementation and tests. The production client supplies the request tuple inspected by these assertions; the probe exercises its getMe caller. No changed behavior depends on a Codex runtime or protocol contract. (extensions/zalo/src/api.ts:20, 5c4c9c956956)
  • Current-main necessity and separate timer cleanup: Current main still contains requireFirstFetchCall. Its changes since the merge base remove fake timer handles in three tests; those edits are separate from this PR’s introduced assertion replacements. (extensions/zalo/src/api.test.ts:75, d4d766d39fb2)
  • Validation and discussion: The complete supplied PR body reports 24/24 API and probe cases passing before and after, unchanged case inventories, and 19 selected checks passing against the merge base. These are contributor-reported results; this read-only review did not execute tests. Independent git diff --check passed. (extensions/zalo/src/api.test.ts, 5c4c9c956956)

Likely related people:

  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • Alix-007: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete steipete self-assigned this Sep 5, 2026
@steipete
steipete merged commit 4e21793 into main Sep 5, 2026
97 checks passed
@steipete
steipete deleted the codex/test-zalo-first-fetch-assertion branch September 5, 2026 19:05
@steipete

steipete commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

@github-actions github-actions Bot mentioned this pull request Sep 5, 2026
Open
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Sep 6, 2026
@steipete
refactor(test): reuse the Zalo mock-call assertion helper (openclaw#1…
734c4f6 
…39294)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@steipete steipete

Labels

channel: zalo Channel integration: zalo maintainer Maintainer-authored PR P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: XS status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@steipete