-
Notifications
You must be signed in to change notification settings - Fork 37
[Disk Manager] Prepare cells storage for metrics #4728
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Note This is an automated comment that will be appended during run. 🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit ce6c4c3.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit ce6c4c3.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit ce6c4c3.
|
|
Note This is an automated comment that will be appended during run. 🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 613fcce.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 613fcce.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 613fcce.
|
|
Note This is an automated comment that will be appended during run. 🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 45c3f99.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 45c3f99.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 45c3f99.
|
|
Note This is an automated comment that will be appended during run. 🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 7d3a70f.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 7d3a70f.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 7d3a70f.
|
|
Note This is an automated comment that will be appended during run. 🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 32e1cce.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 32e1cce.
🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 32e1cce.
|
|
|
||
| // Uses WithinDuration to compare CreatedAt field correctly, since == may fail | ||
| // due to different internal representations. | ||
| func requireClusterCapacitiesAreEqual( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В комменте отписал, конечно, но на всякий: таймстемпы сравнивать через require.Equal нельзя, т.к. после записи в БД они чуть-чуть меняются
| capacity1, | ||
| cellCapacity1, | ||
| }, capacities) | ||
| sort.Slice(capacities, func(i, j int) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Порядок получения из GetRecentClusterCapacitites не детерминирован. Имхо, ради тестов его фиксировать бессмысленно. Так что проще отсортировать на месте
|
|
||
| //////////////////////////////////////////////////////////////////////////////// | ||
|
|
||
| func matchClusterCapacities(t *testing.T, expected cells_storage.ClusterCapacity) interface{} { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Теперь CreatedAt задаём в коде таски, нужно сверять время. Обычно в таких случаях мы просто подставляем mock.Anything, но тут терялся бы смысл теста.
В матчере сравниваем полученное время с time.Now(), т.к. такой же time.Now() выставляется в коде таски. Минута про запас
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
| expected.TotalBytes != got.TotalBytes || | ||
| expected.CellID != got.CellID || expected.Kind != got.Kind || | ||
| expected.ZoneID != got.ZoneID || | ||
| time.Now().Sub(got.CreatedAt).Abs() >= time.Minute { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Давай сначала сделаем проверку на время, потом приравняем время и сравним expected и got целиком на равенство? А то если в структуру потом добавится новое поле, то тут легко забыть добавить проверку.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Логично. Исправил в обоих тестах
#4729
Preparing the cells storage for metrics. Removing redundant structure
clusterCapacityState, making CreatedAt field public incells.ClusterCapacity