Skip to content

fix: align TrackNodeSend test with actual TrackNode API#1246

Merged
ChuxiJ merged 1 commit intomainfrom
claude/fix-failing-pr-tests-SRT8M
Mar 30, 2026
Merged

fix: align TrackNodeSend test with actual TrackNode API#1246
ChuxiJ merged 1 commit intomainfrom
claude/fix-failing-pr-tests-SRT8M

Conversation

@ChuxiJ
Copy link
Copy Markdown

@ChuxiJ ChuxiJ commented Mar 30, 2026

Summary

  • Fix TrackNodeSend.test.ts calling non-existent updateSendPrePost() method — use updateSendAmount() instead
  • Fix all connectSend() calls to use boolean (true/false) instead of string literals ('pre'/'post')
  • Test was written against a planned API that diverged from the final implementation in PR feat: wire aux sends to audio engine with pre/post fader toggle #1039

Test plan

  • All 6 TrackNodeSend tests pass
  • Full test suite passes (3476 tests, 0 failures)
  • No production code changes (test-only fix)

Closes #1245

https://claude.ai/code/session_017CJfg9zq4GvRCuPaSWvHKW

Copilot AI review requested due to automatic review settings March 30, 2026 05:25
Copy link
Copy Markdown

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

Updates the TrackNodeSend unit tests to match the actual TrackNode aux-send API (as implemented in PR #1039), unblocking CI by removing calls to a non-existent method.

Changes:

  • Replace updateSendPrePost() usage with updateSendAmount(id, amount, preFader) in TrackNodeSend.test.ts.
  • Update connectSend() calls to pass preFader: boolean (true/false) instead of 'pre' | 'post'.
  • Adjust one test description/comment to reflect the updated API intent.
Comments suppressed due to low confidence (1)

src/engine/tests/TrackNodeSend.test.ts:120

  • The comment here says the pre-fader tap point should be sumGain, but TrackNode.connectSend() actually taps preFaderOutput (compressor or latency-comp node) before volumeGain. Also, the test currently has no assertions—updating the comment and asserting which node performed the connect() call would make this test meaningful.
    trackNode.connectSend('send-1', sendDest, 0.5, true);

    // The pre-fader tap point should be the sumGain (before volumeGain)
    // Verify by checking that sumGain.connect was called (for the send gain node)
    // This is an integration-level check — the send gain should connect from sumGain

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +109 to +111
trackNode.connectSend('send-1', sendDest, 0.5, false);
trackNode.updateSendAmount('send-1', 0.5, true);
// No throw = success; pre/post switching happens via gain crossfade
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

This test case doesn't assert any observable behavior (it will pass as long as no exception is thrown), even though the name suggests it validates switching from post- to pre-fader. Consider asserting the internal send gain state (e.g., pre gain ramps to amount and post gain ramps to 0) similar to src/engine/__tests__/TrackNode.sends.test.ts so the test actually verifies the tap-point switch.

Suggested change
trackNode.connectSend('send-1', sendDest, 0.5, false);
trackNode.updateSendAmount('send-1', 0.5, true);
// No throw = success; pre/post switching happens via gain crossfade
const amount = 0.5;
trackNode.connectSend('send-1', sendDest, amount, false);
trackNode.updateSendAmount('send-1', amount, true);
// Inspect all gain nodes created by the mock AudioContext
const gainCreationResults = (ctx.createGain as unknown as ReturnType<typeof vi.fn>).mock
.results;
const gainNodes = gainCreationResults.map((r: { value: any }) => r.value);
// Collect all linearRampToValueAtTime target values across all gain nodes
const rampCalls = gainNodes.flatMap((node: any) =>
(node.gain.linearRampToValueAtTime as ReturnType<typeof vi.fn>).mock.calls,
);
const rampTargets = rampCalls.map((args: any[]) => args[0]);
// Verify that we actually scheduled ramps
expect(rampTargets.length).toBeGreaterThan(0);
// One gain should ramp up to the send amount (pre-fader), another down to 0 (post-fader)
expect(rampTargets).toContain(amount);
expect(rampTargets).toContain(0);

Copilot uses AI. Check for mistakes.
The test was calling `updateSendPrePost()` which doesn't exist.
The actual API uses `updateSendAmount(id, amount, preFader: boolean)`
for both amount changes and pre/post switching. Also fix `connectSend`
calls to use boolean `preFader` parameter instead of string literals.

https://claude.ai/code/session_017CJfg9zq4GvRCuPaSWvHKW
@ChuxiJ ChuxiJ force-pushed the claude/fix-failing-pr-tests-SRT8M branch from 6ec96aa to c52ed5e Compare March 30, 2026 06:32
@ChuxiJ ChuxiJ merged commit f1ceb03 into main Mar 30, 2026
2 of 3 checks passed
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.

fix: TrackNodeSend test calls non-existent updateSendPrePost method

3 participants