test(agentdrain): migrate miner_test.go to testify, add coverage for TrainEvent/Clusters/Coordinator/persistence

[Copilot]

miner_test.go used raw t.Fatal/t.Errorf throughout while its sibling anomaly_test.go already uses testify. This brings the file in line with package conventions and adds coverage for previously untested code paths.

Refactoring

• Replaced all t.Fatal/t.Fatalf setup checks with require.NoError / require.NotNil
• Replaced all t.Errorf comparisons with assert.Equal, assert.Contains, assert.InDelta, assert.Positive, assert.Error
• Replaced manual slice-loop in TestMergeTemplate with assert.Equal
• Collapsed TestTrain_ClusterCreation + TestTrain_ClusterMerge into a single table-driven TestTrain (4 subtests: create, no-merge, separate clusters, merge→wildcard)
• All assertions carry descriptive messages

New tests

Test	What it covers
TestTrainEvent	Stage propagation through Miner.TrainEvent
TestClusters	Miner.Clusters() snapshot on empty and non-empty miner
TestCoordinatorAnalyzeEvent	First-seen → IsNewTemplate; repeat → not new; unknown stage → error
TestStageSequence	Table-driven: empty, single, multi-event sequences
TestPersistenceRoundTrip	SaveSnapshots → LoadSnapshots preserves per-stage cluster counts

[github-actions]

Hey @Copilot 👋 — great work on the miner_test.go migration! Bringing this file in line with the testify patterns already established in anomaly_test.go makes the package's test suite consistent and much easier to read. The new coverage for TrainEvent, Clusters, CoordinatorAnalyzeEvent, StageSequence, and the persistence round-trip fills in some meaningful gaps.

This PR looks well-structured and ready for maintainer review. 🎉

Generated by Contribution Check · ● 1.8M · ◷

[Copilot]

Pull request overview

Updates pkg/agentdrain/miner_test.go to use testify assertions consistently (matching existing package test conventions) and adds new tests to cover previously untested coordinator/miner behaviors, including event training/routing, stage sequencing, and snapshot persistence.

Changes:

• Migrated existing tests from manual t.Fatal/t.Errorf patterns to require/assert.
• Refactored training-related tests into a single table-driven TestTrain and added new tests for TrainEvent, Clusters, and coordinator AnalyzeEvent.
• Added persistence round-trip coverage via SaveSnapshots/LoadSnapshots and expanded stage-sequence coverage.

Show a summary per file

File	Description
pkg/agentdrain/miner_test.go	Migrates tests to testify and adds new coverage for training, coordinator analysis, stage sequencing, and persistence.

Copilot's findings

Tip

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

Comments suppressed due to low confidence (3)

pkg/agentdrain/miner_test.go:178

• indexIn does not compile: for i := range len(s) - len(substr) + 1 attempts to range over an int. Use strings.Index (and re-add the strings import) or a classic for i := 0; i <= len(s)-len(substr); i++ loop with a guard for len(substr) > len(s).

func indexIn(s, substr string) int {
	for i := range len(s) - len(substr) + 1 {
		if s[i:i+len(substr)] == substr {
			return i
		}

pkg/agentdrain/miner_test.go:195

• This test won’t compile: for g := range goroutines / for i := range linesEach attempt to range over int constants. Use a counted loop (for g := 0; g < goroutines; g++ { ... }, similarly for i).

	for g := range goroutines {
		wg.Add(1)
		go func(id int) {
			defer wg.Done()
			for i := range linesEach {

pkg/agentdrain/miner_test.go:199

• assert.NoError(t, ...) is called from multiple goroutines; testing.T (and testify assertions that wrap it) are not safe to use concurrently and can race/panic. Collect errors in a channel/slice protected by a mutex and assert in the main goroutine after wg.Wait(), or use t.Run subtests instead of raw goroutines.

				line := fmt.Sprintf("stage=work goroutine=%d iter=%d", id, i)
				_, trainErr := m.Train(line)
				assert.NoError(t, trainErr, "Train should not error during concurrent access")
			}

• Files reviewed: 1/1 changed files
• Comments generated: 1

[Copilot]

The key-order assertion can pass even if a key is missing because indexIn returns -1 and -1 < someIndex is true. Assert each required key exists (index != -1) before comparing order, or use strings.Index and explicitly check for -1.

This issue also appears in the following locations of the same file:

• line 174
• line 191
• line 196

Suggested change

	assert.True(t, len(result) > 0 &&
	indexIn(result, "latency_ms=") < indexIn(result, "query=") &&
	indexIn(result, "query=") < indexIn(result, "tool="),
	latencyIdx := indexIn(result, "latency_ms=")
	queryIdx := indexIn(result, "query=")
	toolIdx := indexIn(result, "tool=")
	assert.NotEqual(t, -1, latencyIdx, "flattened output should contain latency_ms: %q", result)
	assert.NotEqual(t, -1, queryIdx, "flattened output should contain query: %q", result)
	assert.NotEqual(t, -1, toolIdx, "flattened output should contain tool: %q", result)
	assert.True(t, len(result) > 0 &&
	latencyIdx < queryIdx &&
	queryIdx < toolIdx,