[Disk Manager] Fix flappy test for snapshot service #3120
Conversation
Rattysed
changed the title
Rattysed
added
large-tests
| @@ -377,7 +378,20 @@ func WaitForCheckpointsDoNotExist( | |||
| checkpoints, err := nbsClient.GetCheckpoints(ctx, diskID) | |||
| require.NoError(t, err) | |||
|
|
|||
| if len(checkpoints) == 0 { | |||
| checkpointSet := make(map[string]struct{}, len(checkpoints)) | |||
There was a problem hiding this comment.
Думаю, получается слишком сложный код на ровном месте.
Может оставить WaitForCheckpointsDoNotExist как был, и написать отдельный метод на ожидание конкретных чекпоинтов? Мб даже не передавать туда слайс чекпоинтов, а только один чекпоинт.
There was a problem hiding this comment.
Тоже думал так сделать. А если нам понадобится ждать 2 чекпоинта, то сделать ещё один метод, ожидающий 2 чекпоинта?
Конечно, такое маловероятно, но всё же получаем копипасту
| // If snapshot creation ends up successfuly, there may be two cases: | ||
| // Snapshot was created and then deleted, so should be no checkpoints left | ||
| // or snapshot deletion ended up before creation, snapshot was not deleted, | ||
| // so there should be a checkpoint. | ||
| if creationErr != nil { |
There was a problem hiding this comment.
чет я не понял комментария
ты пишешь про случае когда снапшот успешно создался
а иф проверяет то что создание с ошибой упало
There was a problem hiding this comment.
Пишу как раз чтобы объяснить, почему этот случай не проверяем. Для случая с ошибкой комментарий внутри
| snapshotID, _, err := testcommon.GetIncremental(ctx, &types.Disk{ | ||
| ZoneId: "zone-a", | ||
| DiskId: diskID, | ||
| }) | ||
| require.NoError(t, err) | ||
|
|
||
| // Should wait here because checkpoint is deleted on |createOperation| |
There was a problem hiding this comment.
не понятный коммент
что за |createOperation| operation?
во-первых - какая-то тавтология
во-вторых - что это за операция такая? может ты имел в виду createSnapshot?
There was a problem hiding this comment.
Сверху есть переменная createOperation, на ошибку от которой мы и смотрим. Про тавтологию согласен
cloud/disk_manager/internal/pkg/facade/snapshot_service_test/snapshot_service_test.go
Outdated
Show resolved
Hide resolved
cloud/disk_manager/internal/pkg/facade/snapshot_service_test/snapshot_service_test.go
Show resolved
Hide resolved
One of the tests affected by #2008 was not correct.
One of it's flaps: https://github.com/ydb-platform/nbs/actions/runs/13579009298
Also changed WaitForCheckpointsDoNotExist method for better testing