Skip to content

Commit d14c7fc

Browse files
committed
fix tests
Signed-off-by: Alex Wu <c.alexwu@gmail.com>
1 parent 83db8e2 commit d14c7fc

3 files changed

Lines changed: 26 additions & 16 deletions

File tree

actions/k8s/recovery_test.go

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,6 @@ func TestResolveRecoveredFrom_HitStampsSourceResult(t *testing.T) {
110110
runClient, c := newGateTestClient(t)
111111
runClient.EXPECT().LookupAction(mock.Anything, mock.Anything).
112112
Return(lookupResponse(&workflow.LookupActionResponse{
113-
Found: true,
114113
Phase: common.ActionPhase_ACTION_PHASE_SUCCEEDED,
115114
Attempts: 2,
116115
CacheStatus: core.CatalogCacheStatus_CACHE_HIT,
@@ -141,7 +140,6 @@ func TestResolveRecoveredFrom_SignalledConditionIsAHit(t *testing.T) {
141140
}
142141
runClient.EXPECT().LookupAction(mock.Anything, mock.Anything).
143142
Return(lookupResponse(&workflow.LookupActionResponse{
144-
Found: true,
145143
Phase: common.ActionPhase_ACTION_PHASE_SUCCEEDED,
146144
Output: signal,
147145
}), nil).Once()
@@ -163,7 +161,6 @@ func TestResolveRecoveredFrom_RecoveredSourceIsAHit(t *testing.T) {
163161
runClient, c := newGateTestClient(t)
164162
runClient.EXPECT().LookupAction(mock.Anything, mock.Anything).
165163
Return(lookupResponse(&workflow.LookupActionResponse{
166-
Found: true,
167164
Phase: common.ActionPhase_ACTION_PHASE_RECOVERED,
168165
OutputUri: "s3://bucket/r0/a1/1/outputs.pb",
169166
}), nil).Once()
@@ -175,20 +172,32 @@ func TestResolveRecoveredFrom_RecoveredSourceIsAHit(t *testing.T) {
175172
assert.Equal(t, "s3://bucket/r0/a1/1/outputs.pb", got.OutputUri)
176173
}
177174

178-
func TestResolveRecoveredFrom_MissAndUnusableSourcesRunFresh(t *testing.T) {
175+
// A source run that never had this action reports NOT_FOUND. That is an ordinary recovery
176+
// outcome, not a lookup failure, so it runs fresh without touching the failure counter —
177+
// the distinction TestResolveRecoveredFrom_LookupFailureRunsFresh covers from the other side.
178+
func TestResolveRecoveredFrom_MissingActionRunsFresh(t *testing.T) {
179+
runClient, c := newGateTestClient(t)
180+
runClient.EXPECT().LookupAction(mock.Anything, mock.Anything).
181+
Return(nil, connect.NewError(connect.CodeNotFound, assert.AnError)).Once()
182+
183+
got := c.resolveRecoveredFrom(context.Background(),
184+
taskActionWith(recoveryContextFor("r1")), childAction("a1"), false)
185+
assert.Nil(t, got)
186+
}
187+
188+
func TestResolveRecoveredFrom_UnusableSourcesRunFresh(t *testing.T) {
179189
for _, tc := range []struct {
180190
name string
181191
resp *workflow.LookupActionResponse
182192
}{
183-
{"missing", &workflow.LookupActionResponse{Found: false}},
184193
{"failed", &workflow.LookupActionResponse{
185-
Found: true, Phase: common.ActionPhase_ACTION_PHASE_FAILED, OutputUri: "s3://x",
194+
Phase: common.ActionPhase_ACTION_PHASE_FAILED, OutputUri: "s3://x",
186195
}},
187196
{"aborted", &workflow.LookupActionResponse{
188-
Found: true, Phase: common.ActionPhase_ACTION_PHASE_ABORTED, OutputUri: "s3://x",
197+
Phase: common.ActionPhase_ACTION_PHASE_ABORTED, OutputUri: "s3://x",
189198
}},
190199
{"succeeded without outputs", &workflow.LookupActionResponse{
191-
Found: true, Phase: common.ActionPhase_ACTION_PHASE_SUCCEEDED,
200+
Phase: common.ActionPhase_ACTION_PHASE_SUCCEEDED,
192201
}},
193202
} {
194203
t.Run(tc.name, func(t *testing.T) {

executor/test/integration/setup_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ package integration
2222

2323
import (
2424
"context"
25+
"errors"
2526
"log"
2627
"os"
2728
"path/filepath"
@@ -222,9 +223,9 @@ func (r *recordingRunClient) RecordActionEvents(_ context.Context, _ *connect.Re
222223
return connect.NewResponse(&workflow.RecordActionEventsResponse{}), nil
223224
}
224225

225-
// No run under test is a recovery, so every lookup is a miss.
226+
// No run under test is a recovery, so every lookup is a miss — reported as NOT_FOUND.
226227
func (r *recordingRunClient) LookupAction(_ context.Context, _ *connect.Request[workflow.LookupActionRequest]) (*connect.Response[workflow.LookupActionResponse], error) {
227-
return connect.NewResponse(&workflow.LookupActionResponse{Found: false}), nil
228+
return nil, connect.NewError(connect.CodeNotFound, errors.New("no recovery source under test"))
228229
}
229230

230231
func (r *recordingRunClient) RecordActionStream(_ context.Context) *connect.BidiStreamForClient[workflow.RecordActionStreamRequest, workflow.RecordActionStreamResponse] {

runs/service/recovery_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -123,17 +123,19 @@ func lookupActionID(run, name string) *common.ActionIdentifier {
123123
return &common.ActionIdentifier{Run: sourceRunID(run), Name: name}
124124
}
125125

126-
func TestLookupAction_MissingActionIsNotAnError(t *testing.T) {
126+
// A miss is NOT_FOUND, and the caller keys its fail-open path off exactly that code
127+
// (actions/k8s/recovery.go), so the code itself is the contract — not just "an error".
128+
func TestLookupAction_MissingActionIsNotFound(t *testing.T) {
127129
actionRepo, svc := newRecoveryTestService(t)
128130
actionID := lookupActionID("r1", "a5")
129131

130132
actionRepo.On("GetAction", mock.Anything, matchActionID(actionID)).
131133
Return(nil, interfaces.ErrActionNotFound).Once()
132134

133-
resp, err := svc.LookupAction(context.Background(),
135+
_, err := svc.LookupAction(context.Background(),
134136
connect.NewRequest(&workflow.LookupActionRequest{ActionId: actionID}))
135-
require.NoError(t, err)
136-
assert.False(t, resp.Msg.GetFound())
137+
require.Error(t, err)
138+
assert.Equal(t, connect.CodeNotFound, connect.CodeOf(err))
137139
}
138140

139141
// A failing lookup must stay distinguishable from a miss: the caller counts them separately.
@@ -170,7 +172,6 @@ func TestLookupAction_TaskActionReadsLastAttemptOutputs(t *testing.T) {
170172
resp, err := svc.LookupAction(context.Background(),
171173
connect.NewRequest(&workflow.LookupActionRequest{ActionId: actionID}))
172174
require.NoError(t, err)
173-
assert.True(t, resp.Msg.GetFound())
174175
assert.Equal(t, common.ActionPhase_ACTION_PHASE_SUCCEEDED, resp.Msg.GetPhase())
175176
assert.Equal(t, uint32(2), resp.Msg.GetAttempts())
176177
assert.Equal(t, core.CatalogCacheStatus_CACHE_HIT, resp.Msg.GetCacheStatus())
@@ -245,7 +246,6 @@ func TestLookupAction_NoOutputsYieldsEmptyURI(t *testing.T) {
245246
resp, err := svc.LookupAction(context.Background(),
246247
connect.NewRequest(&workflow.LookupActionRequest{ActionId: actionID}))
247248
require.NoError(t, err)
248-
assert.True(t, resp.Msg.GetFound())
249249
assert.Empty(t, resp.Msg.GetOutputUri())
250250
}
251251

0 commit comments

Comments
 (0)