From 320210194b6e2e19d09ace1691e68289b1cbf58a Mon Sep 17 00:00:00 2001 From: hickford Date: Mon, 8 Aug 2022 15:01:56 +0100 Subject: [PATCH] Fix order-dependent test Be careful to initialise counters before taking snapshot --- CHANGELOG.md | 1 + log/operation_manager_test.go | 10 ++++++---- monitoring/testonly/delta.go | 3 +++ 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c4313cda0..c8f6b8395e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ (`*`) by default. * Updated golangci-lint to v1.47.3 (developers should update to this version) * Delete merkle package, use [github.com/transparency-dev/merkle](https://pkg.go.dev/github.com/transparency-dev/merkle) instead. +* #2792: Fixed order-dependent unit test. ## v1.4.2 diff --git a/log/operation_manager_test.go b/log/operation_manager_test.go index 4ea1fd4da7..4ce6d5765b 100644 --- a/log/operation_manager_test.go +++ b/log/operation_manager_test.go @@ -166,14 +166,15 @@ func TestOperationManagerExecutePassError(t *testing.T) { mockLogOp.EXPECT().ExecutePass(gomock.Any(), logID1, infoMatcher).Return(1, nil) mockLogOp.EXPECT().ExecutePass(gomock.Any(), logID2, infoMatcher).Return(0, errors.New("test error")) + info := defaultOperationInfo(registry) + lom := NewOperationManager(info, mockLogOp) // initialises counters + // Do not run this test in parallel with any other that affects the signingRuns or failedSigningRuns counters. log1SigningRuns := testonly.NewCounterSnapshot(signingRuns, logID1Label) log1FailedSigningRuns := testonly.NewCounterSnapshot(failedSigningRuns, logID1Label) log2SigningRuns := testonly.NewCounterSnapshot(signingRuns, logID2Label) log2FailedSigningRuns := testonly.NewCounterSnapshot(failedSigningRuns, logID2Label) - info := defaultOperationInfo(registry) - lom := NewOperationManager(info, mockLogOp) lom.OperationSingle(ctx) // Check that logID1 has 1 successful signing run, 0 failed. @@ -318,14 +319,15 @@ func TestOperationManagerOperationLoopExecutePassError(t *testing.T) { } }).Return(0, errors.New("test error")) + info := defaultOperationInfo(registry) + lom := NewOperationManager(info, mockLogOp) // initialises counters + // Do not run this test in parallel with any other that affects the signingRuns or failedSigningRuns counters. log1SigningRuns := testonly.NewCounterSnapshot(signingRuns, logID1Label) log1FailedSigningRuns := testonly.NewCounterSnapshot(failedSigningRuns, logID1Label) log2SigningRuns := testonly.NewCounterSnapshot(signingRuns, logID2Label) log2FailedSigningRuns := testonly.NewCounterSnapshot(failedSigningRuns, logID2Label) - info := defaultOperationInfo(registry) - lom := NewOperationManager(info, mockLogOp) lom.OperationLoop(ctx) // Check that logID1 has 1 successful signing run, 0 failed. diff --git a/monitoring/testonly/delta.go b/monitoring/testonly/delta.go index c7c78c5acc..4fa25f81bc 100644 --- a/monitoring/testonly/delta.go +++ b/monitoring/testonly/delta.go @@ -34,6 +34,9 @@ type CounterSnapshot struct { // by the given labels. This value can be compared to future values to determine // how it has changed over time. func NewCounterSnapshot(c monitoring.Counter, labels ...string) CounterSnapshot { + if c == nil { + panic("can't take snapshot of nil counter") + } return CounterSnapshot{ c: c, labels: labels,