Skip to content

test(cloud): mock ad accounts as active in credit-reconciliation suite — un-red develop after #11619 gate - #11715

Merged
lalalune merged 1 commit into
developfrom
fix/ad-reconciliation-gate-mocks
Jul 3, 2026
Merged

lalalune merged 1 commit into
developfrom
fix/ad-reconciliation-gate-mocks

Conversation

@lalalune

@lalalune lalalune commented Jul 3, 2026

Copy link
Copy Markdown
Member

What

#11619 gated updateCampaign on account.status === "active" (closing the suspended-account spend leg from #11364) but did not update the mocks in packages/cloud/shared/src/lib/services/__tests__/ad-campaign-credit-reconciliation.test.ts, which stub adAccountsRepository.findById without a status field.

Since that merge, 7 of the suite's 13 tests fail on develop with:

error: Ad account is not active (status: undefined); it must be approved before running campaigns
      at updateCampaign (packages/cloud/shared/src/lib/services/advertising/index.ts:696)

— the gate throws before the credit-hold reconciliation logic under test is ever reached.

Fix

Add status: "active" to the two beforeEach account mocks so the suite exercises budget-change hold reconciliation again. The approval gate itself remains covered by ad-account-approval.test.ts (15 tests, unchanged).

Evidence

On origin/develop @ 80ba667fd0 (pre-fix):

bun test packages/cloud/shared/src/lib/services/__tests__/ad-campaign-credit-reconciliation.test.ts
 6 pass
 7 fail   # all: 'Ad account is not active (status: undefined)'

With this fix (same base):

bun test .../ad-campaign-credit-reconciliation.test.ts .../ad-account-approval.test.ts .../ad-inventory.test.ts .../ad-tag-token.test.ts
 48 pass
 0 fail

Frontend/video/screenshot/trajectory evidence: N/A — test-only change to existing unit-test mocks; no runtime surface.

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Jul 3, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on this repository. 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e81352d9-e0c7-462e-a29f-53f46b3ef980

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/ad-reconciliation-gate-mocks

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.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

…e — un-red develop after #11619 gate

#11619 gated updateCampaign on account.status === "active" but did not
update the ad-campaign-credit-reconciliation mocks, which stub
adAccountsRepository.findById without a status field. Since that merge,
7 of the suite's 13 tests fail on develop with 'Ad account is not active
(status: undefined)' before ever reaching the credit logic under test.

Mark both beforeEach account mocks active so the suite exercises hold
reconciliation again; the gate itself stays covered by
ad-account-approval.test.ts (15 tests).

Before: 6 pass / 7 fail. After: 13 pass / 0 fail (48/0 across the four
ad suites).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@lalalune
lalalune force-pushed the fix/ad-reconciliation-gate-mocks branch from 96f1849 to a8f0b15 Compare July 3, 2026 00:37

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

@lalalune

lalalune commented Jul 3, 2026

Copy link
Copy Markdown
Member Author

Validation after rebase onto current origin/develop:

  • Confirmed the diff is only the two missing status: "active" fields in adAccountsRepository.findById mocks for the reconciliation suite.
  • Initial raw bun test ... run reached the expected passing test output but ended with Bun WriteFailed while dumping the huge coverage table, so I reran with the compact reporter.

Checks run:

  • bun test --coverage-reporter=lcov --reporter=dots packages/cloud/shared/src/lib/services/__tests__/ad-campaign-credit-reconciliation.test.ts packages/cloud/shared/src/lib/services/__tests__/ad-account-approval.test.ts packages/cloud/shared/src/lib/services/__tests__/ad-inventory.test.ts packages/cloud/shared/src/lib/services/__tests__/ad-tag-token.test.ts → 48 pass / 0 fail
  • bun run --cwd packages/cloud/shared typecheck
  • bunx @biomejs/biome@2.5.2 check packages/cloud/shared/src/lib/services/__tests__/ad-campaign-credit-reconciliation.test.ts --files-ignore-unknown=true
  • git diff --check origin/develop...HEAD && git diff --check

No approval from me because this PR is self-authored by lalalune.

@lalalune
lalalune merged commit e59ddaf into develop Jul 3, 2026
35 of 46 checks passed
@lalalune
lalalune deleted the fix/ad-reconciliation-gate-mocks branch July 3, 2026 00:39
@github-actions github-actions Bot added the Tests label Jul 3, 2026
@claude

claude Bot commented Jul 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude encountered an error —— View job


I'll analyze this and get back to you.

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.

2 participants