Skip to content

Commit 5f82693

Browse files
test(covenantsigner): decouple scriptedEngine from SignerApprovalVerifier interface
scriptedEngine implicitly implemented SignerApprovalVerifier via its signerApprovalVerifier field and VerifySignerApproval method. This caused NewService to auto-wire a signerApprovalVerifier on every test service, triggering the validation guard that requires SignerApproval in the request whenever a verifier is configured. Tests using baseRequest (no SignerApproval) then failed before calling the engine, hanging goroutine synchronization channels indefinitely. Remove VerifySignerApproval and the signerApprovalVerifier field from scriptedEngine. Tests that need a signer approval verifier now pass it explicitly via WithSignerApprovalVerifier, making the intent clear.
1 parent 7ef4b7e commit 5f82693

1 file changed

Lines changed: 28 additions & 44 deletions

File tree

pkg/covenantsigner/covenantsigner_test.go

Lines changed: 28 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -154,11 +154,10 @@ func (cfh *contentFaultingHandle) ReadAll() (<-chan persistence.DataDescriptor,
154154
}
155155

156156
type scriptedEngine struct {
157-
submit func(*Job) (*Transition, error)
158-
poll func(*Job) (*Transition, error)
159-
currentBlockHeight uint64
160-
currentBlockErr error
161-
signerApprovalVerifier SignerApprovalVerifier
157+
submit func(*Job) (*Transition, error)
158+
poll func(*Job) (*Transition, error)
159+
currentBlockHeight uint64
160+
currentBlockErr error
162161
}
163162

164163
func (se *scriptedEngine) OnSubmit(_ context.Context, job *Job) (*Transition, error) {
@@ -179,13 +178,6 @@ func (se *scriptedEngine) CurrentBlockHeight(context.Context) (uint64, error) {
179178
return se.currentBlockHeight, se.currentBlockErr
180179
}
181180

182-
func (se *scriptedEngine) VerifySignerApproval(request RouteSubmitRequest) error {
183-
if se.signerApprovalVerifier == nil {
184-
return nil
185-
}
186-
return se.signerApprovalVerifier.VerifySignerApproval(request)
187-
}
188-
189181
func mustJSON(t *testing.T, value any) []byte {
190182
t.Helper()
191183
data, err := json.Marshal(value)
@@ -935,10 +927,9 @@ func TestServiceSubmitDeduplicatesByRouteRequestID(t *testing.T) {
935927
submit: func(*Job) (*Transition, error) {
936928
return &Transition{State: JobStatePending, Detail: "queued"}, nil
937929
},
938-
signerApprovalVerifier: SignerApprovalVerifierFunc(
939-
func(RouteSubmitRequest) error { return nil },
940-
),
941-
})
930+
}, WithSignerApprovalVerifier(SignerApprovalVerifierFunc(
931+
func(RouteSubmitRequest) error { return nil },
932+
)))
942933
if err != nil {
943934
t.Fatal(err)
944935
}
@@ -973,10 +964,9 @@ func TestServiceSubmitRejectsRouteRequestIDDigestMismatch(t *testing.T) {
973964
submit: func(*Job) (*Transition, error) {
974965
return &Transition{State: JobStatePending, Detail: "queued"}, nil
975966
},
976-
signerApprovalVerifier: SignerApprovalVerifierFunc(
977-
func(RouteSubmitRequest) error { return nil },
978-
),
979-
})
967+
}, WithSignerApprovalVerifier(SignerApprovalVerifierFunc(
968+
func(RouteSubmitRequest) error { return nil },
969+
)))
980970
if err != nil {
981971
t.Fatal(err)
982972
}
@@ -1016,10 +1006,9 @@ func TestServiceSubmitReturnsExistingJobWhileInitialEngineCallIsInFlight(t *test
10161006
<-releaseEngine
10171007
return &Transition{State: JobStatePending, Detail: "queued"}, nil
10181008
},
1019-
signerApprovalVerifier: SignerApprovalVerifierFunc(
1020-
func(RouteSubmitRequest) error { return nil },
1021-
),
1022-
})
1009+
}, WithSignerApprovalVerifier(SignerApprovalVerifierFunc(
1010+
func(RouteSubmitRequest) error { return nil },
1011+
)))
10231012
if err != nil {
10241013
t.Fatal(err)
10251014
}
@@ -3007,9 +2996,6 @@ func TestServicePollAcceptsEquivalentArtifactApprovalRequestVariants(t *testing.
30072996
return &Transition{State: JobStatePending, Detail: "polling"}, nil
30082997
},
30092998
currentBlockHeight: 100,
3010-
signerApprovalVerifier: SignerApprovalVerifierFunc(func(request RouteSubmitRequest) error {
3011-
return nil
3012-
}),
30132999
})
30143000
if err != nil {
30153001
t.Fatal(err)
@@ -3439,11 +3425,10 @@ func TestServicePollPropagatesCurrentBlockProviderError(t *testing.T) {
34393425
},
34403426
currentBlockHeight: 100,
34413427
currentBlockErr: wantErr,
3442-
signerApprovalVerifier: SignerApprovalVerifierFunc(func(request RouteSubmitRequest) error {
3443-
return nil
3444-
}),
34453428
}
3446-
service, err := NewService(handle, engine, WithCurrentBlockProvider(engine))
3429+
service, err := NewService(handle, engine, WithCurrentBlockProvider(engine), WithSignerApprovalVerifier(SignerApprovalVerifierFunc(func(request RouteSubmitRequest) error {
3430+
return nil
3431+
})))
34473432
if err != nil {
34483433
t.Fatal(err)
34493434
}
@@ -3489,11 +3474,10 @@ func TestServiceLoadPollJobPropagatesCurrentBlockProviderError(t *testing.T) {
34893474
},
34903475
currentBlockHeight: 100,
34913476
currentBlockErr: wantErr,
3492-
signerApprovalVerifier: SignerApprovalVerifierFunc(func(request RouteSubmitRequest) error {
3493-
return nil
3494-
}),
34953477
}
3496-
service, err := NewService(handle, engine, WithCurrentBlockProvider(engine))
3478+
service, err := NewService(handle, engine, WithCurrentBlockProvider(engine), WithSignerApprovalVerifier(SignerApprovalVerifierFunc(func(request RouteSubmitRequest) error {
3479+
return nil
3480+
})))
34973481
if err != nil {
34983482
t.Fatal(err)
34993483
}
@@ -3536,12 +3520,11 @@ func TestServicePollRejectsExpiredCertificate(t *testing.T) {
35363520
return &Transition{State: JobStatePending, Detail: "polling"}, nil
35373521
},
35383522
currentBlockHeight: 200, // EndBlock is 123456; set >= EndBlock to trigger expiration
3539-
signerApprovalVerifier: SignerApprovalVerifierFunc(func(request RouteSubmitRequest) error {
3540-
return nil
3541-
}),
35423523
}
35433524
engine.currentBlockHeight = 123456 // satisfy expiration: currentBlock >= EndBlock
3544-
service, err := NewService(handle, engine, WithCurrentBlockProvider(engine))
3525+
service, err := NewService(handle, engine, WithCurrentBlockProvider(engine), WithSignerApprovalVerifier(SignerApprovalVerifierFunc(func(request RouteSubmitRequest) error {
3526+
return nil
3527+
})))
35453528
if err != nil {
35463529
t.Fatal(err)
35473530
}
@@ -3582,10 +3565,9 @@ func TestServicePollAcceptsValidCertificate(t *testing.T) {
35823565
return &Transition{State: JobStatePending, Detail: "polling"}, nil
35833566
},
35843567
currentBlockHeight: 100, // EndBlock is 123456, so currentBlock < EndBlock
3585-
signerApprovalVerifier: SignerApprovalVerifierFunc(func(request RouteSubmitRequest) error {
3586-
return nil
3587-
}),
3588-
})
3568+
}, WithSignerApprovalVerifier(SignerApprovalVerifierFunc(func(request RouteSubmitRequest) error {
3569+
return nil
3570+
})))
35893571
if err != nil {
35903572
t.Fatal(err)
35913573
}
@@ -3625,7 +3607,9 @@ func TestServicePollSkipsBlockProviderWhenNoExpiration(t *testing.T) {
36253607
return &Transition{State: JobStatePending, Detail: "polling"}, nil
36263608
},
36273609
currentBlockHeight: 100,
3628-
})
3610+
}, WithSignerApprovalVerifier(SignerApprovalVerifierFunc(func(RouteSubmitRequest) error {
3611+
return nil
3612+
})))
36293613
if err != nil {
36303614
t.Fatal(err)
36313615
}

0 commit comments

Comments
 (0)