Skip to content

test(amber): add unit test coverage for EmptyReplayLogManagerImpl#4790

Merged
Yicong-Huang merged 3 commits into
apache:mainfrom
aglinxinyuan:xinyuan-test-empty-replay-log-manager-spec
May 3, 2026
Merged

test(amber): add unit test coverage for EmptyReplayLogManagerImpl#4790
Yicong-Huang merged 3 commits into
apache:mainfrom
aglinxinyuan:xinyuan-test-empty-replay-log-manager-spec

Conversation

@aglinxinyuan
Copy link
Copy Markdown
Contributor

@aglinxinyuan aglinxinyuan commented May 3, 2026

What changes were proposed in this PR?

Add EmptyReplayLogManagerImplSpec covering the no-op replay-log manager and the inherited withFaultTolerant bookkeeping:

  • getStep starts at INIT_STEP
  • setupWriter / markAsReplayDestination / terminate are no-ops (exercised with real fixtures — EmptyRecordStorage[ReplayLogRecord]().getWriter("x") and EmbeddedControlMessageIdentity("test") — instead of null)
  • sendCommitted forwards the message to the configured handler
  • withFaultTolerant advances the step counter (INIT_STEP + 1, then INIT_STEP + 2) after the body runs
  • withFaultTolerant advances the step counter and rethrows when the body throws

Any related issues, documentation, discussions?

Closes #4789

How was this PR tested?

sbt "WorkflowExecutionService/testOnly org.apache.texera.amber.engine.architecture.logreplay.EmptyReplayLogManagerImplSpec" — 5/5 tests pass.

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Claude Opus 4.7)

Add EmptyReplayLogManagerImplSpec covering the no-op replay-log manager
used when fault tolerance is disabled: initial step is INIT_STEP,
setupWriter/markAsReplayDestination/terminate are no-ops, sendCommitted
forwards to the handler, and the inherited withFaultTolerant advances
the step counter (including on exception).

Closes apache#4789

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 3, 2026 01:54
@github-actions github-actions Bot added the engine label May 3, 2026
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

Adds ScalaTest unit coverage for EmptyReplayLogManagerImpl, the no-op replay-log manager used when fault tolerance is disabled, including verification of inherited ReplayLogManager.withFaultTolerant step bookkeeping behavior.

Changes:

  • Introduces EmptyReplayLogManagerImplSpec to validate initial cursor step and that no-op lifecycle methods don’t affect observable state.
  • Adds coverage that sendCommitted forwards committed messages to the configured handler.
  • Adds coverage that withFaultTolerant increments the processing step both on success and when the body throws (rethrowing the exception).

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

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 43.21%. Comparing base (6485b04) to head (4d11c97).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #4790      +/-   ##
============================================
+ Coverage     43.19%   43.21%   +0.01%     
- Complexity     2035     2037       +2     
============================================
  Files           957      957              
  Lines         34077    34077              
  Branches       3753     3753              
============================================
+ Hits          14721    14727       +6     
+ Misses        18580    18576       -4     
+ Partials        776      774       -2     
Flag Coverage Δ
amber 41.37% <ø> (+0.03%) ⬆️
python 84.84% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@aglinxinyuan aglinxinyuan requested a review from Yicong-Huang May 3, 2026 01:59
- Use real fixtures (an `EmptyRecordStorage` writer and an
  `EmbeddedControlMessageIdentity("test")`) instead of `null` when
  exercising `setupWriter` / `markAsReplayDestination`, so the test
  reflects realistic call sites.
- Express step assertions relative to `ProcessingStepCursor.INIT_STEP`
  (`+1`, `+2`) instead of hardcoded `0L` / `1L`, so the spec stays
  correct if the initial-step constant ever changes.
- Use `{}` block syntax for empty `withFaultTolerant` bodies.
- Apply scalafmt.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Yicong-Huang Yicong-Huang enabled auto-merge (squash) May 3, 2026 07:17
@Yicong-Huang Yicong-Huang merged commit 7b6ed23 into apache:main May 3, 2026
15 checks passed
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.

add unit test coverage for EmptyReplayLogManagerImpl

4 participants