Skip to content

Commit

Permalink
Fix flaky test due to package-level test variable set in a separate t…
Browse files Browse the repository at this point in the history 
…est (#17393)
  • Loading branch information
@mna
mna authored Mar 13, 2024
1 parent 55c7f1e commit 8d8181e
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 63 deletions.
49 changes: 19 additions & 30 deletions server/datastore/mysql/apple_mdm.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1177,17 +1177,6 @@ func (ds *Datastore) BatchSetMDMAppleProfiles(ctx context.Context, tmID *uint, p
})
}

// set this in tests to simulate an error at various stages in the
// batchSetMDMAppleProfilesDB execution: if the string starts with "insert", it
// will be in the insert/upsert stage, "delete" for deletion, "select" to load
// existing ones, "reselect" to reload existing ones after insert, and "labels"
// to simulate an error in batch setting the profile label associations.
// "inselect", "inreselect", "indelete", etc. can also be used to fail the
// sqlx.In before the corresponding statement.
//
// e.g.: testBatchSetMDMAppleProfilesErr = "insert:fail"
var testBatchSetMDMAppleProfilesErr string

// batchSetMDMAppleProfilesDB must be called from inside a transaction.
func (ds *Datastore) batchSetMDMAppleProfilesDB(
ctx context.Context,
Expand Down Expand Up @@ -1252,15 +1241,15 @@ ON DUPLICATE KEY UPDATE
if len(incomingIdents) > 0 {
// load existing profiles that match the incoming profiles by identifiers
stmt, args, err := sqlx.In(loadExistingProfiles, profTeamID, incomingIdents)
if err != nil || strings.HasPrefix(testBatchSetMDMAppleProfilesErr, "inselect") {
if err != nil || strings.HasPrefix(ds.testBatchSetMDMAppleProfilesErr, "inselect") {
if err == nil {
err = errors.New(testBatchSetMDMAppleProfilesErr)
err = errors.New(ds.testBatchSetMDMAppleProfilesErr)
}
return ctxerr.Wrap(ctx, err, "build query to load existing profiles")
}
if err := sqlx.SelectContext(ctx, tx, &existingProfiles, stmt, args...); err != nil || strings.HasPrefix(testBatchSetMDMAppleProfilesErr, "select") {
if err := sqlx.SelectContext(ctx, tx, &existingProfiles, stmt, args...); err != nil || strings.HasPrefix(ds.testBatchSetMDMAppleProfilesErr, "select") {
if err == nil {
err = errors.New(testBatchSetMDMAppleProfilesErr)
err = errors.New(ds.testBatchSetMDMAppleProfilesErr)
}
return ctxerr.Wrap(ctx, err, "load existing profiles")
}
Expand All @@ -1287,24 +1276,24 @@ ON DUPLICATE KEY UPDATE
)
// delete the obsolete profiles (all those that are not in keepIdents or delivered by Fleet)
stmt, args, err = sqlx.In(deleteProfilesNotInList, profTeamID, append(keepIdents, fleetIdents...))
if err != nil || strings.HasPrefix(testBatchSetMDMAppleProfilesErr, "indelete") {
if err != nil || strings.HasPrefix(ds.testBatchSetMDMAppleProfilesErr, "indelete") {
if err == nil {
err = errors.New(testBatchSetMDMAppleProfilesErr)
err = errors.New(ds.testBatchSetMDMAppleProfilesErr)
}
return ctxerr.Wrap(ctx, err, "build statement to delete obsolete profiles")
}
if _, err := tx.ExecContext(ctx, stmt, args...); err != nil || strings.HasPrefix(testBatchSetMDMAppleProfilesErr, "delete") {
if _, err := tx.ExecContext(ctx, stmt, args...); err != nil || strings.HasPrefix(ds.testBatchSetMDMAppleProfilesErr, "delete") {
if err == nil {
err = errors.New(testBatchSetMDMAppleProfilesErr)
err = errors.New(ds.testBatchSetMDMAppleProfilesErr)
}
return ctxerr.Wrap(ctx, err, "delete obsolete profiles")
}

// insert the new profiles and the ones that have changed
for _, p := range incomingProfs {
if _, err := tx.ExecContext(ctx, insertNewOrEditedProfile, profTeamID, p.Identifier, p.Name, p.Mobileconfig); err != nil || strings.HasPrefix(testBatchSetMDMAppleProfilesErr, "insert") {
if _, err := tx.ExecContext(ctx, insertNewOrEditedProfile, profTeamID, p.Identifier, p.Name, p.Mobileconfig); err != nil || strings.HasPrefix(ds.testBatchSetMDMAppleProfilesErr, "insert") {
if err == nil {
err = errors.New(testBatchSetMDMAppleProfilesErr)
err = errors.New(ds.testBatchSetMDMAppleProfilesErr)
}
return ctxerr.Wrapf(ctx, err, "insert new/edited profile with identifier %q", p.Identifier)
}
Expand All @@ -1319,15 +1308,15 @@ ON DUPLICATE KEY UPDATE
var newlyInsertedProfs []*fleet.MDMAppleConfigProfile
// load current profiles (again) that match the incoming profiles by name to grab their uuids
stmt, args, err := sqlx.In(loadExistingProfiles, profTeamID, incomingIdents)
if err != nil || strings.HasPrefix(testBatchSetMDMAppleProfilesErr, "inreselect") {
if err != nil || strings.HasPrefix(ds.testBatchSetMDMAppleProfilesErr, "inreselect") {
if err == nil {
err = errors.New(testBatchSetMDMAppleProfilesErr)
err = errors.New(ds.testBatchSetMDMAppleProfilesErr)
}
return ctxerr.Wrap(ctx, err, "build query to load newly inserted profiles")
}
if err := sqlx.SelectContext(ctx, tx, &newlyInsertedProfs, stmt, args...); err != nil || strings.HasPrefix(testBatchSetMDMAppleProfilesErr, "reselect") {
if err := sqlx.SelectContext(ctx, tx, &newlyInsertedProfs, stmt, args...); err != nil || strings.HasPrefix(ds.testBatchSetMDMAppleProfilesErr, "reselect") {
if err == nil {
err = errors.New(testBatchSetMDMAppleProfilesErr)
err = errors.New(ds.testBatchSetMDMAppleProfilesErr)
}
return ctxerr.Wrap(ctx, err, "load newly inserted profiles")
}
Expand All @@ -1346,9 +1335,9 @@ ON DUPLICATE KEY UPDATE
}

// insert label associations
if err := batchSetProfileLabelAssociationsDB(ctx, tx, incomingLabels, "darwin"); err != nil || strings.HasPrefix(testBatchSetMDMAppleProfilesErr, "labels") {
if err := batchSetProfileLabelAssociationsDB(ctx, tx, incomingLabels, "darwin"); err != nil || strings.HasPrefix(ds.testBatchSetMDMAppleProfilesErr, "labels") {
if err == nil {
err = errors.New(testBatchSetMDMAppleProfilesErr)
err = errors.New(ds.testBatchSetMDMAppleProfilesErr)
}
return ctxerr.Wrap(ctx, err, "inserting apple profile label associations")
}
Expand Down Expand Up @@ -2908,7 +2897,7 @@ func updateHostDEPAssignProfileResponses(ctx context.Context, tx sqlx.ExtContext
stmt := `
UPDATE
host_dep_assignments
JOIN
JOIN
hosts ON id = host_id
SET
profile_uuid = ?,
Expand Down Expand Up @@ -2952,8 +2941,8 @@ SELECT
FROM
host_dep_assignments
JOIN hosts ON id = host_id
WHERE
hardware_serial IN (?)
WHERE
hardware_serial IN (?)
`

stmt, args, err := sqlx.In(stmt, string(fleet.DEPAssignProfileResponseFailed), depCooldownPeriod.Seconds(), serials)
Expand Down
8 changes: 4 additions & 4 deletions server/datastore/mysql/mdm_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3063,8 +3063,8 @@ func testBatchSetMDMProfilesTransactionError(t *testing.T, ds *Datastore) {
for _, c := range cases {
t.Run(c.windowsErr+" "+c.appleErr, func(t *testing.T) {
t.Cleanup(func() {
testBatchSetMDMAppleProfilesErr = ""
testBatchSetMDMWindowsProfilesErr = ""
ds.testBatchSetMDMAppleProfilesErr = ""
ds.testBatchSetMDMWindowsProfilesErr = ""
})

appleProfs := []*fleet.MDMAppleConfigProfile{
Expand All @@ -3089,8 +3089,8 @@ func testBatchSetMDMProfilesTransactionError(t *testing.T, ds *Datastore) {
windowsConfigProfileForTest(t, "W3", "l3", lbl),
}
// setup the expected errors
testBatchSetMDMAppleProfilesErr = c.appleErr
testBatchSetMDMWindowsProfilesErr = c.windowsErr
ds.testBatchSetMDMAppleProfilesErr = c.appleErr
ds.testBatchSetMDMWindowsProfilesErr = c.windowsErr

err = ds.BatchSetMDMProfiles(ctx, nil, appleProfs, winProfs)
require.ErrorContains(t, err, c.wantErr)
Expand Down
47 changes: 18 additions & 29 deletions server/datastore/mysql/microsoft_mdm.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1587,17 +1587,6 @@ ON DUPLICATE KEY UPDATE
return nil
}

// set this in tests to simulate an error at various stages in the
// batchSetMDMWindowsProfilesDB execution: if the string starts with "insert",
// it will be in the insert/upsert stage, "delete" for deletion, "select" to
// load existing ones, "reselect" to reload existing ones after insert, and
// "labels" to simulate an error in batch setting the profile label
// associations. "inselect", "inreselect", "indelete", etc. can also be used to
// fail the sqlx.In before the corresponding statement.
//
// e.g.: testBatchSetMDMWindowsProfilesErr = "insert:fail"
var testBatchSetMDMWindowsProfilesErr string

// batchSetMDMWindowsProfilesDB must be called from inside a transaction.
func (ds *Datastore) batchSetMDMWindowsProfilesDB(
ctx context.Context,
Expand Down Expand Up @@ -1668,15 +1657,15 @@ ON DUPLICATE KEY UPDATE
if len(incomingNames) > 0 {
// load existing profiles that match the incoming profiles by name
stmt, args, err := sqlx.In(loadExistingProfiles, profTeamID, incomingNames)
if err != nil || strings.HasPrefix(testBatchSetMDMWindowsProfilesErr, "inselect") {
if err != nil || strings.HasPrefix(ds.testBatchSetMDMWindowsProfilesErr, "inselect") {
if err == nil {
err = errors.New(testBatchSetMDMWindowsProfilesErr)
err = errors.New(ds.testBatchSetMDMWindowsProfilesErr)
}
return ctxerr.Wrap(ctx, err, "build query to load existing profiles")
}
if err := sqlx.SelectContext(ctx, tx, &existingProfiles, stmt, args...); err != nil || strings.HasPrefix(testBatchSetMDMWindowsProfilesErr, "select") {
if err := sqlx.SelectContext(ctx, tx, &existingProfiles, stmt, args...); err != nil || strings.HasPrefix(ds.testBatchSetMDMWindowsProfilesErr, "select") {
if err == nil {
err = errors.New(testBatchSetMDMWindowsProfilesErr)
err = errors.New(ds.testBatchSetMDMWindowsProfilesErr)
}
return ctxerr.Wrap(ctx, err, "load existing profiles")
}
Expand All @@ -1698,32 +1687,32 @@ ON DUPLICATE KEY UPDATE
// delete the obsolete profiles (all those that are not in keepNames)
if len(keepNames) > 0 {
stmt, args, err = sqlx.In(deleteProfilesNotInList, profTeamID, keepNames)
if err != nil || strings.HasPrefix(testBatchSetMDMWindowsProfilesErr, "indelete") {
if err != nil || strings.HasPrefix(ds.testBatchSetMDMWindowsProfilesErr, "indelete") {
if err == nil {
err = errors.New(testBatchSetMDMWindowsProfilesErr)
err = errors.New(ds.testBatchSetMDMWindowsProfilesErr)
}
return ctxerr.Wrap(ctx, err, "build statement to delete obsolete profiles")
}
if _, err := tx.ExecContext(ctx, stmt, args...); err != nil || strings.HasPrefix(testBatchSetMDMWindowsProfilesErr, "delete") {
if _, err := tx.ExecContext(ctx, stmt, args...); err != nil || strings.HasPrefix(ds.testBatchSetMDMWindowsProfilesErr, "delete") {
if err == nil {
err = errors.New(testBatchSetMDMWindowsProfilesErr)
err = errors.New(ds.testBatchSetMDMWindowsProfilesErr)
}
return ctxerr.Wrap(ctx, err, "delete obsolete profiles")
}
} else {
if _, err := tx.ExecContext(ctx, deleteAllProfilesForTeam, profTeamID); err != nil || strings.HasPrefix(testBatchSetMDMWindowsProfilesErr, "delete") {
if _, err := tx.ExecContext(ctx, deleteAllProfilesForTeam, profTeamID); err != nil || strings.HasPrefix(ds.testBatchSetMDMWindowsProfilesErr, "delete") {
if err == nil {
err = errors.New(testBatchSetMDMWindowsProfilesErr)
err = errors.New(ds.testBatchSetMDMWindowsProfilesErr)
}
return ctxerr.Wrap(ctx, err, "delete all profiles for team")
}
}

// insert the new profiles and the ones that have changed
for _, p := range incomingProfs {
if _, err := tx.ExecContext(ctx, insertNewOrEditedProfile, profTeamID, p.Name, p.SyncML); err != nil || strings.HasPrefix(testBatchSetMDMWindowsProfilesErr, "insert") {
if _, err := tx.ExecContext(ctx, insertNewOrEditedProfile, profTeamID, p.Name, p.SyncML); err != nil || strings.HasPrefix(ds.testBatchSetMDMWindowsProfilesErr, "insert") {
if err == nil {
err = errors.New(testBatchSetMDMWindowsProfilesErr)
err = errors.New(ds.testBatchSetMDMWindowsProfilesErr)
}
return ctxerr.Wrapf(ctx, err, "insert new/edited profile with name %q", p.Name)
}
Expand All @@ -1738,15 +1727,15 @@ ON DUPLICATE KEY UPDATE
var newlyInsertedProfs []*fleet.MDMWindowsConfigProfile
// load current profiles (again) that match the incoming profiles by name to grab their uuids
stmt, args, err := sqlx.In(loadExistingProfiles, profTeamID, incomingNames)
if err != nil || strings.HasPrefix(testBatchSetMDMWindowsProfilesErr, "inreselect") {
if err != nil || strings.HasPrefix(ds.testBatchSetMDMWindowsProfilesErr, "inreselect") {
if err == nil {
err = errors.New(testBatchSetMDMWindowsProfilesErr)
err = errors.New(ds.testBatchSetMDMWindowsProfilesErr)
}
return ctxerr.Wrap(ctx, err, "build query to load newly inserted profiles")
}
if err := sqlx.SelectContext(ctx, tx, &newlyInsertedProfs, stmt, args...); err != nil || strings.HasPrefix(testBatchSetMDMWindowsProfilesErr, "reselect") {
if err := sqlx.SelectContext(ctx, tx, &newlyInsertedProfs, stmt, args...); err != nil || strings.HasPrefix(ds.testBatchSetMDMWindowsProfilesErr, "reselect") {
if err == nil {
err = errors.New(testBatchSetMDMWindowsProfilesErr)
err = errors.New(ds.testBatchSetMDMWindowsProfilesErr)
}
return ctxerr.Wrap(ctx, err, "load newly inserted profiles")
}
Expand All @@ -1765,9 +1754,9 @@ ON DUPLICATE KEY UPDATE
}

// insert/delete the label associations
if err := batchSetProfileLabelAssociationsDB(ctx, tx, incomingLabels, "windows"); err != nil || strings.HasPrefix(testBatchSetMDMWindowsProfilesErr, "labels") {
if err := batchSetProfileLabelAssociationsDB(ctx, tx, incomingLabels, "windows"); err != nil || strings.HasPrefix(ds.testBatchSetMDMWindowsProfilesErr, "labels") {
if err == nil {
err = errors.New(testBatchSetMDMWindowsProfilesErr)
err = errors.New(ds.testBatchSetMDMWindowsProfilesErr)
}
return ctxerr.Wrap(ctx, err, "inserting windows profile label associations")
}
Expand Down
22 changes: 22 additions & 0 deletions server/datastore/mysql/mysql.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,28 @@ type Datastore struct {
testDeleteMDMProfilesBatchSize int
// for tests, set to override the default batch size.
testUpsertMDMDesiredProfilesBatchSize int

// set this in tests to simulate an error at various stages in the
// batchSetMDMAppleProfilesDB execution: if the string starts with "insert", it
// will be in the insert/upsert stage, "delete" for deletion, "select" to load
// existing ones, "reselect" to reload existing ones after insert, and "labels"
// to simulate an error in batch setting the profile label associations.
// "inselect", "inreselect", "indelete", etc. can also be used to fail the
// sqlx.In before the corresponding statement.
//
// e.g.: testBatchSetMDMAppleProfilesErr = "insert:fail"
testBatchSetMDMAppleProfilesErr string

// set this in tests to simulate an error at various stages in the
// batchSetMDMWindowsProfilesDB execution: if the string starts with "insert",
// it will be in the insert/upsert stage, "delete" for deletion, "select" to
// load existing ones, "reselect" to reload existing ones after insert, and
// "labels" to simulate an error in batch setting the profile label
// associations. "inselect", "inreselect", "indelete", etc. can also be used to
// fail the sqlx.In before the corresponding statement.
//
// e.g.: testBatchSetMDMWindowsProfilesErr = "insert:fail"
testBatchSetMDMWindowsProfilesErr string
}

// reader returns the DB instance to use for read-only statements, which is the
Expand Down

0 comments on commit 8d8181e

Please sign in to comment.