test(batching): added more test coverage to check update transaction optimisations

[krutibaraiya]

Test cases added

batcher_test.go — 7 tests

Test	Scenario
TestBatcher_TimerFlush	Pending updates are flushed automatically when the flush interval timer fires
TestBatcher_MaxBatchSizeFlush	Flush is triggered immediately when the batch reaches maxBatchSize before the timer
TestBatcher_StopFlushes	Stop() drains and processes all pending updates before shutting down
TestBatcher_DeduplicationInQueue	Adding the same check multiple times keeps only the latest update (no duplicate ops)
TestBatcher_AddAfterStop	Calling Add() on a stopped batcher does not panic
TestBatcher_ConcurrentAdds	Concurrent Add() calls from multiple goroutines don't race or corrupt state
TestFlushLocked_ProcessFuncNil	flushLocked() is a no-op and doesn't panic when processFunc is nil

---

agentless_optimization_test.go — 4 tests

Test	Scenario
TestCheckRunner_AgentlessOptimizations	Agentful runner has batcher=nil; agentless runner has batcher initialised and isAgentless=true
TestCheckRunner_MaxBatchSize	The maxTxnOps constant is 64 (Consul's transaction limit)
TestCheckRunner_StateReversion	revertCheckState() correctly rolls back local check state; handles missing checks gracefully; enables natural retry by diverging local vs actual state
TestCheckRunner_OriginalStateInBatcher	batcher.Add() stores oldStatus/oldOutput alongside the new status so rollback is possible on failure

---

check_transaction_integration_test.go — 22 tests (against a live Consul test server)

Test	Scenario
TestExecuteBatchTransaction_Success	Happy path: CAS transaction updates a check's status and is verified in Consul
TestExecuteBatchTransaction_StaleIndex	CAS fails with stale ModifyIndex, triggering retry which succeeds
TestExecuteBatchTransaction_NetworkError	Client pointing at an invalid address — network error handled gracefully without panic
TestExecuteBatchTransaction_NonStaleErrors	Transaction returns a non-CAS error (empty node name) — no retry attempted
TestExecuteBatchTransaction_EmptyOps	Calling with empty/nil ops returns immediately without error
TestProcessBatchedUpdates_NilClient	Runner with client=nil returns early without panic
TestProcessBatchedUpdates_EmptyUpdates	Empty and nil update slices return early without error
TestProcessBatchedUpdates_NodeFetchError	Update for a non-existent node — fetch error handled gracefully
TestProcessBatchedUpdates_CheckNoLongerExists	Check deleted before batch processes — skipped without error
TestRevertCheckState_MissingCheck	revertCheckState() with a check not in the local store — no panic
TestHashCheck_WithoutServiceID	Node-level check hashes as node/checkID
TestHashCheck_WithServiceID	Service-level check hashes as node/serviceID/checkID
TestBatcher_NewCheckUpdateBatcher_ZeroMaxSize	Zero MaxBatchSize in config falls back to the maxTxnOps default
TestRetryFailedBatchOperations_CheckDeleted	Retry skips gracefully when the check has been deregistered from Consul
TestRetryFailedBatchOperations_InvalidIndex	Error references an OpIndex out of bounds — handled without panic
TestRetryFailedBatchOperations_NodeFetchError	Server stopped mid-retry — node fetch error handled gracefully
TestRetryFailedBatchOperations_RetryFails	Stale index persists after retry (check updated multiple times externally) — state reverted via revertCheckState
TestRetryFailedBatchOperations_SuccessfulRetry	Stale CAS error on first attempt; retry fetches fresh index and succeeds
TestRetryFailedBatchOperations_EmptyErrors	Empty TxnErrors slice passed to retry — returns immediately
TestRetryFailedBatchOperations_CheckDeletedDuringRetry	Check exists when retry starts but is deleted before the retry transaction — handled gracefully
TestRetryFailedBatchOperations_RetryTransactionError	Retry transaction itself returns an error (non-stale) — error logged, state reverted
TestHandleCheckUpdate_AgentfulMode	Agentful mode (isAgentless=false) takes the handleCheckUpdateImmediate path instead of batching

PCI review checklist

• I have documented a clear reason for, and description of, the change I am making.

• If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.

• If applicable, I've documented the impact of any changes to security controls.

  Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.

[Copilot]

Pull request overview

Adds test coverage around agentless batching and Consul transaction retry behavior to validate recent update-transaction optimizations and edge cases.

Changes:

• Added a large integration-style test suite covering batched txn execution and retry paths (stale index, missing checks, nil client, etc.).
• Added unit tests for CheckUpdateBatcher flush behavior (timer flush, max-size flush, deduplication, stop behavior, concurrency).
• Removed older “concept/demo” batching tests from agentless_optimization_test.go.

Reviewed changes

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

File	Description
check_transaction_integration_test.go	New integration tests exercising batched transactions and retry/revert edge cases.
batcher_test.go	New unit tests for batcher flushing/dedup/Stop and concurrent Add behavior.
agentless_optimization_test.go	Removes placeholder batching tests, keeping focused optimization/state tests.

---

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

You can also share your feedback on Copilot code review. Take the survey.