Skip to content

test: add unit test coverage for src/jobs/ - #831

Merged
manan19 merged 3 commits into
mainfrom
test/jobs-coverage
Apr 25, 2026
Merged

test: add unit test coverage for src/jobs/#831
manan19 merged 3 commits into
mainfrom
test/jobs-coverage

Conversation

@manan19

@manan19 manan19 commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Summary

The src/jobs/ directory had zero test coverage despite running production-critical pruning, snapshot upload, and on-chain event migration logic. Adds 13 inline #[cfg(test)] tests across all four modules:

  • block_pruning (4 tests) — job creation succeeds in both sync states; sync gate semantics (skips when not synced, runs when synced)
  • event_pruning (3 tests) — job creation; cutoff timestamp = farcaster_time - retention.as_secs(); longer retention prunes further back
  • snapshot_upload (4 tests) — UploadAlreadyInProgress returned when backup dir has recent contents; empty only_for_shard_ids filter skips all backups; stale-backup contents cleaned up when mtime is older than 12h; STALE_BACKUP_THRESHOLD constant is exactly 12 hours
  • migrate_onchain_events (2 tests) — empty store completes migration immediately (fast exit before wait_for_mempool_to_clear); mempool backpressure waits and recovers when size drops below MAX_MEMPOOL_SIZE

Adds filetime = "0.2" as a [dev-dependencies] entry for the stale-backup test (used to set an old mtime on the temp directory).

Split out from #796 per review feedback.

Test plan

  • cargo test jobs passes (13/13)

🤖 Generated with Claude Code

The src/jobs/ directory had zero test coverage despite running
production-critical pruning, snapshot upload, and migration logic.
Adds 13 inline #[cfg(test)] tests across all four modules:

- block_pruning: job creation in both sync states + sync gate semantics
- event_pruning: job creation + cutoff timestamp math
- snapshot_upload: in-progress detection, empty shard filter,
  stale-backup cleanup (uses filetime to set old mtime), threshold
  constant
- migrate_onchain_events: empty-store fast exit, mempool backpressure
  via wait_for_mempool_to_clear

Adds filetime as a dev-dependency for the stale-backup test.

Split out from #796 per review feedback.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 24, 2026 23:35
@vercel

vercel Bot commented Apr 24, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
snapchain-docs Ready Ready Preview, Comment Apr 25, 2026 4:33am

Request Review

Copilot AI 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.

Pull request overview

Adds unit tests to the production-critical src/jobs/ modules to increase coverage around pruning, snapshot upload behavior, and on-chain event migration/backpressure handling.

Changes:

  • Added inline #[cfg(test)] tests to block_pruning, event_pruning, snapshot_upload, and migrate_onchain_events.
  • Added filetime as a dev-dependency to support filesystem mtime manipulation in snapshot stale-backup tests.
  • Updated Cargo.lock to reflect the new dev-dependency resolution.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/jobs/snapshot_upload.rs Adds tests for in-progress detection, shard filtering, stale-backup cleanup, and the 12h threshold constant.
src/jobs/migrate_onchain_events.rs Adds tests for empty-store fast-exit and mempool backpressure waiting behavior.
src/jobs/event_pruning.rs Adds tests for job creation and retention/cutoff timestamp arithmetic.
src/jobs/block_pruning.rs Adds tests for job creation under sync states and “sync gate” assertions.
Cargo.toml Adds filetime = "0.2" under [dev-dependencies].
Cargo.lock Updates lockfile entries after adding filetime dev-dependency.

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

Comment thread src/jobs/event_pruning.rs
Comment thread src/jobs/event_pruning.rs
Comment thread src/jobs/block_pruning.rs Outdated
Comment thread src/jobs/block_pruning.rs Outdated
@github-actions

github-actions Bot commented Apr 24, 2026

Copy link
Copy Markdown

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

  • src/jobs/block_pruning.rs (84.4%): Missing lines 39-44,46-47,121,140
  • src/jobs/event_pruning.rs (88.9%): Missing lines 22,25,46
  • src/jobs/migrate_onchain_events.rs (96.8%): Missing lines 268,270
  • src/jobs/snapshot_upload.rs (100%)

Summary

  • Total: 246 lines
  • Missing: 15 lines
  • Coverage: 93%

src/jobs/block_pruning.rs

  35         let sync_complete_rx = sync_complete_rx.clone();
  36         let block_stores = block_stores.clone();
  37         let shard_stores = shard_stores.clone();
  38         Box::pin(async move {
! 39             let cutoff_timestamp = match pruning_cutoff(
! 40                 util::get_farcaster_time().unwrap(),
! 41                 block_retention,
! 42                 *sync_complete_rx.borrow(),
! 43             ) {
! 44                 Some(cutoff) => cutoff,
  45                 None => {
! 46                     info!("Sync not complete, skipping block pruning");
! 47                     return;
  48                 }
  49             };
  50 
  51             let stop_height = block_stores

  117         );
  118         assert!(
  119             result.is_ok(),
  120             "expected job creation to succeed: {:?}",
! 121             result.err()
  122         );
  123     }
  124 
  125     #[test]

  136         );
  137         assert!(
  138             result.is_ok(),
  139             "expected job creation to succeed: {:?}",
! 140             result.err()
  141         );
  142     }
  143 
  144     #[test]

src/jobs/event_pruning.rs

  18 ) -> Result<Job, JobSchedulerError> {
  19     Job::new_async(schedule, move |_, _| {
  20         let shard_stores = shard_stores.clone();
  21         Box::pin(async move {
! 22             let cutoff = cutoff_timestamp(get_farcaster_time().unwrap(), event_retention);
  23             for (_shard_id, stores) in shard_stores.iter() {
  24                 stores
! 25                     .prune_events_until(cutoff, THROTTLE, None)
  26                     .await
  27                     .unwrap_or_else(|e| {
  28                         error!("Error pruning events: {}", e);
  29                         0

  42         let result = event_pruning_job("0/1 * * * * *", Duration::from_secs(86400), HashMap::new());
  43         assert!(
  44             result.is_ok(),
  45             "expected job creation to succeed: {:?}",
! 46             result.err()
  47         );
  48     }
  49 
  50     #[test]

src/jobs/migrate_onchain_events.rs

  264                         0
  265                     };
  266                     sizes.insert(0u32, size);
  267                     let _ = reply.send(sizes);
! 268                 }
  269             }
! 270         });
  271 
  272         // wait_for_mempool_to_clear is private; test its contract via the public behavior:
  273         // the channel responder above will eventually return a small size, so the call must complete.
  274         tokio::time::timeout(

Per PR review (#831): the original cutoff/sync-gate tests just
re-derived the arithmetic and watch::Receiver semantics, so they could
pass even if the job's behavior changed. Extract the cutoff
computation (and sync gate, for block_pruning) into pure helper
functions used by both the job closures and the tests.

- event_pruning::cutoff_timestamp(now, retention) -> u64
- block_pruning::pruning_cutoff(now, retention, sync_complete) -> Option<u64>
  (returns None when sync is incomplete, Some(cutoff) otherwise)

Both helpers use saturating_sub so a misconfigured retention longer
than the current farcaster time produces 0 instead of underflowing.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot AI 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.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.


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

Comment thread src/jobs/snapshot_upload.rs Outdated
Comment thread src/jobs/migrate_onchain_events.rs Outdated
Per PR review (#831): the test modules in snapshot_upload.rs and
migrate_onchain_events.rs explicitly imported items (RocksDB,
BlockStores, FarcasterNetwork, Arc, HashSet, mpsc, etc.) that are
already in scope via `use super::*;` since the parent module imports
them. Removed the duplicates; kept only items genuinely not in the
parent (Config, MerkleTrie, TempDir, test_helper, StoreLimits).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@manan19
manan19 merged commit 549956e into main Apr 25, 2026
14 checks passed
@manan19
manan19 deleted the test/jobs-coverage branch April 25, 2026 06:08
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