Test error return values from pagers (#4656)

Some code (e.x. Exchange) continues to use the results from a pager even
in the event some errors occur. These tests help ensure we have
consistent return values when we see an error during enumeration

---

#### Does this PR need a docs update or release note?

- [ ]  Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [x]  No

#### Type of change

- [ ] 🌻 Feature
- [ ] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [x] 🤖 Supportability/Tests
- [ ] 💻 CI/Deployment
- [ ] 🧹 Tech Debt/Cleanup

#### Test Plan

- [x] 💪 Manual
- [x]  Unit test
- [ ] 💚 E2E
This commit is contained in:
ashmrtn 2023-11-13 17:01:30 -08:00 committed by GitHub
parent 15418d0dda
commit bb1f287e6a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 110 additions and 14 deletions

View File

@ -532,7 +532,10 @@ func GetAddedAndRemovedItemIDs[T any](
} }
if err != nil && !graph.IsErrInvalidDelta(err) && !graph.IsErrDeltaNotSupported(err) { if err != nil && !graph.IsErrInvalidDelta(err) && !graph.IsErrDeltaNotSupported(err) {
return AddedAndRemoved{}, graph.Stack(ctx, err) return AddedAndRemoved{
DU: DeltaUpdate{Reset: true},
ValidModTimes: deltaPager.ValidModTimes(),
}, graph.Stack(ctx, err)
} else if err == nil { } else if err == nil {
return aar, graph.Stack(ctx, err).OrNil() return aar, graph.Stack(ctx, err).OrNil()
} }

View File

@ -474,9 +474,12 @@ func (suite *PagerUnitSuite) TestGetAddedAndRemovedItemIDs() {
// maxGetterIdx is the maximum index for the item page getter. Helps ensure // maxGetterIdx is the maximum index for the item page getter. Helps ensure
// we're stopping enumeration when we reach the item limit. // we're stopping enumeration when we reach the item limit.
maxGetterIdx int maxGetterIdx int
// expectNoDelta should be set if we exit enumeration early due to the item // noDelta should be set if we exit enumeration early due to the item limit
// limit so we don't get a delta token. // so we don't get a delta token.
noDelta bool noDelta bool
// deltaReset should be set if something specific to the error case will
// cause the delta to be marked as reset (e.x. error).
deltaReset bool
} }
table := []struct { table := []struct {
@ -618,7 +621,30 @@ func (suite *PagerUnitSuite) TestGetAddedAndRemovedItemIDs() {
} }
}, },
expect: expected{ expect: expected{
errCheck: assert.Error, errCheck: assert.Error,
maxGetterIdx: 1,
noDelta: true,
deltaReset: true,
},
},
{
name: "ErrorDeletedInFlight",
pagerGetter: func(t *testing.T, validModTimes bool) *testIDsNonDeltaMultiPager {
return &testIDsNonDeltaMultiPager{
t: t,
pages: []pageResult{
{
err: graph.ErrDeletedInFlight,
},
},
validModTimes: validModTimes,
}
},
expect: expected{
errCheck: assert.Error,
maxGetterIdx: 1,
noDelta: true,
deltaReset: true,
}, },
}, },
{ {
@ -792,7 +818,7 @@ func (suite *PagerUnitSuite) TestGetAddedAndRemovedItemIDs() {
) *testIDsNonDeltaMultiPager { ) *testIDsNonDeltaMultiPager {
return &testIDsNonDeltaMultiPager{ return &testIDsNonDeltaMultiPager{
t: t, t: t,
validModTimes: true, validModTimes: validModTimes,
pages: []pageResult{ pages: []pageResult{
{ {
items: []testItem{ items: []testItem{
@ -806,6 +832,8 @@ func (suite *PagerUnitSuite) TestGetAddedAndRemovedItemIDs() {
}, },
expect: expected{ expect: expected{
errCheck: assert.Error, errCheck: assert.Error,
noDelta: true,
deltaReset: true,
maxGetterIdx: 1, maxGetterIdx: 1,
}, },
ctxCancelled: true, ctxCancelled: true,
@ -850,15 +878,19 @@ func (suite *PagerUnitSuite) TestGetAddedAndRemovedItemIDs() {
filters...) filters...)
test.expect.errCheck(t, err, "getting added and removed item IDs: %+v", clues.ToCore(err)) test.expect.errCheck(t, err, "getting added and removed item IDs: %+v", clues.ToCore(err))
if err != nil { // Check return values even if we get an error as some handlers
return // continue to run when some error types are returned.
}
assert.Len(t, addRemoved.Added, test.expect.numAdded, "number of added items") assert.Len(t, addRemoved.Added, test.expect.numAdded, "number of added items")
assert.GreaterOrEqual(t, test.expect.maxGetterIdx, basePager.pageIdx, "number of pager calls") assert.GreaterOrEqual(t, test.expect.maxGetterIdx, basePager.pageIdx, "number of pager calls")
assert.Equal(t, modTimeTest.validModTimes, addRemoved.ValidModTimes, "valid mod times") assert.Equal(t, modTimeTest.validModTimes, addRemoved.ValidModTimes, "valid mod times")
assert.Equal(t, pagerTypeTest.expectDeltaReset, addRemoved.DU.Reset, "delta reset")
assert.Equal(
t,
test.expect.deltaReset || pagerTypeTest.expectDeltaReset,
addRemoved.DU.Reset,
"delta reset")
if pagerTypeTest.expectNoDelta || test.expect.noDelta { if pagerTypeTest.expectNoDelta || test.expect.noDelta {
assert.Empty(t, addRemoved.DU.URL, "delta link") assert.Empty(t, addRemoved.DU.URL, "delta link")
@ -891,10 +923,11 @@ func (suite *PagerUnitSuite) TestGetAddedAndRemovedItemIDs() {
// cancellation, etc. // cancellation, etc.
func (suite *PagerUnitSuite) TestGetAddedAndRemovedItemIDs_FallbackPagers() { func (suite *PagerUnitSuite) TestGetAddedAndRemovedItemIDs_FallbackPagers() {
type expected struct { type expected struct {
errCheck assert.ErrorAssertionFunc errCheck assert.ErrorAssertionFunc
added []testItem added []testItem
removed []testItem removed []testItem
deltaLink assert.ValueAssertionFunc deltaLink assert.ValueAssertionFunc
invertModTime bool
} }
tests := []struct { tests := []struct {
@ -1044,6 +1077,61 @@ func (suite *PagerUnitSuite) TestGetAddedAndRemovedItemIDs_FallbackPagers() {
deltaLink: assert.Empty, deltaLink: assert.Empty,
}, },
}, },
{
name: "TwoValidPages DeltaNotSupported SwitchModTimeValidity",
pagerGetter: func(
t *testing.T,
validModTimes bool,
) NonDeltaHandler[testItem] {
return &testIDsNonDeltaMultiPager{
t: t,
pages: []pageResult{
{
items: []testItem{
addedItem1,
removedItem1,
},
},
{
items: []testItem{
removedItem2,
addedItem2,
},
},
},
validModTimes: !validModTimes,
}
},
deltaPagerGetter: func(
t *testing.T,
validModTimes bool,
) DeltaHandler[testItem] {
return newDeltaPager(
&testIDsNonDeltaMultiPager{
t: t,
pages: []pageResult{
{
err: graph.ErrDeltaNotSupported,
needsReset: true,
},
},
validModTimes: validModTimes,
})
},
expect: expected{
errCheck: assert.NoError,
added: []testItem{
addedItem1,
addedItem2,
},
removed: []testItem{
removedItem1,
removedItem2,
},
deltaLink: assert.Empty,
invertModTime: true,
},
},
{ {
name: "TwoPages DeltaNotSupportedAtEnd", name: "TwoPages DeltaNotSupportedAtEnd",
pagerGetter: func( pagerGetter: func(
@ -1241,7 +1329,12 @@ func (suite *PagerUnitSuite) TestGetAddedAndRemovedItemIDs_FallbackPagers() {
"getting added and removed item IDs: %+v", "getting added and removed item IDs: %+v",
clues.ToCore(err)) clues.ToCore(err))
assert.Equal(t, modTimeTest.validModTimes, addRemoved.ValidModTimes, "valid mod times") wantModTime := modTimeTest.validModTimes
if test.expect.invertModTime {
wantModTime = !wantModTime
}
assert.Equal(t, wantModTime, addRemoved.ValidModTimes, "valid mod times")
assert.True(t, addRemoved.DU.Reset, "delta reset") assert.True(t, addRemoved.DU.Reset, "delta reset")
test.expect.deltaLink(t, addRemoved.DU.URL, "delta link") test.expect.deltaLink(t, addRemoved.DU.URL, "delta link")