tests: Рефакторинг и добавление новых тестов#52
Merged
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughMade benchmark case constructor accept optional Changes
Sequence Diagram(s)(omitted — changes are primarily tests, refactors and small signature tweak; no multi-component new control flow requiring sequence diagrams) Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
tests/ТестыЗамеров.os (1)
18-25: ПроверьтеВыделеноБайтдо вычисления коэффициента.Если профилирование памяти здесь вернет
0, тест упадет с делением на ноль и спрячeт реальную причину сбоя. Лучше сначала сделать явную проверку, а потом считать коэффициент.💡 Небольшое усиление диагностики
// Проверка ВыделеноБайт = Результат.Запуски[0].Статистика.ВыделяемаяПамять; + Ожидаем.Что(ВыделеноБайт, "Выделяемая память").Больше(0); Коэффициент = РазмерВБайтах / ВыделеноБайт; Ожидаем.Что(Коэффициент).Между(0.99, 1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыЗамеров.os` around lines 18 - 25, Перед вычислением Коэффициент убедитесь, что ВыделеноБайт (из Результат.Запуски[0].Статистика.ВыделяемаяПамять) не равен нулю: добавьте явную проверку (например, Ожидаем.Что(ВыделеноБайт).Больше(0) или эквивалентное утверждение) сразу после получения ВыделеноБайт и только затем вычисляйте Коэффициент = РазмерВБайтах / ВыделеноБайт; это предотвратит деление на ноль и сделает причину падения теста очевидной.tests/ТестыСценарияЗапускаБенчмарка.os (1)
25-30: Используйте один экземпляр дескриптора и для сценария, и для кейсов.
КейсПустогоБенчмарка()каждый раз создает новуюКоллекцияДескрипторовБенчмарков, поэтому кейсы в этих тестах не связаны с той коллекцией, которая передана вСценарийЗапускаБенчмарка. В итоге здесь хорошо покрывается сериализация идентификаторов, но хуже ловятся ошибки сопоставленияКейсБенчмарка↔ДескрипторБенчмарка. Надежнее передавать в helper уже созданныйДескрипторБенчмарка.Also applies to: 45-50, 66-73, 96-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыСценарияЗапускаБенчмарка.os` around lines 25 - 30, The test creates separate КоллекцияДескрипторовБенчмарков instances for the СценарийЗапускаБенчмарка and for each helper КейсПустогоБенчмарка, so cases are not tied to the scenario's descriptor collection; fix it by creating a single КоллекцияДескрипторовБенчмарков/ДескрипторБенчмарка instance and passing that same descriptor collection or descriptor into both the СценарийЗапускаБенчмарка constructor and into the helper that builds КейсБенчмарка (or change КейсПустогоБенчмарка to accept a descriptor parameter), and update the other test locations (lines ~45-50, 66-73, 96-100) to reuse the same descriptor instance.tests/ТестыИсполнителяБенчмарков.os (1)
118-121: Minor: Inconsistent indentation in method chain.Line 119 is missing a tab before
.УстановитьСтратегиюcompared to the similar pattern in lines 79-81.🔧 Suggested fix for consistent indentation
Конфигурация - .УстановитьСтратегию(СтратегииЗапускаБенчмарка.ХолодныйЗапуск) + .УстановитьСтратегию(СтратегииЗапускаБенчмарка.ХолодныйЗапуск) .УстановитьКоличествоИтераций(1) .ДобавитьМониторингПамяти();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыИсполнителяБенчмарков.os` around lines 118 - 121, The method chain starting at Конфигурация has inconsistent indentation: add a leading tab before `.УстановитьСтратегию(СтратегииЗапускаБенчмарка.ХолодныйЗапуск)` so the `.УстановитьСтратегию`, `.УстановитьКоличествоИтераций(1)` and `.ДобавитьМониторингПамяти()` calls align with the existing chained call style used elsewhere; update the indentation for those chained methods to match the pattern used on lines 79–81.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/ТестыПодготовителяФайлаБенчмарка.os`:
- Around line 11-20: Текущие проверки ИмеетДлину(0)/ИмеетДлину(1) зависят от
глобального состояния модуля (_МенеджерВременныхФайлов), потому что менеджер
создаётся в ПередВсеми() и живёт до ПослеВсех(); исправьте тесты, чтобы
проверять дельту созданных временных файлов вместо абсолютной длины: в тесте
захватите текущее количество файлов из _МенеджерВременныхФайлов перед вызовом
методов ПодготовительФайлаБенчмарка, выполните операцию и затем сравните разницу
(новая длина минус старая) с ожидаемым значением; альтернативно, адаптируйте
ПодготовленныйФайл/ПодготовительФайлаБенчмарка чтобы возвращал количество или
список созданных временных файлов и проверяйте это значение напрямую (исходные
точки: функции/процедуры _МенеджерВременныхФайлов, ПодготовительФайлаБенчмарка,
и места проверок ИмеетДлину в тестах).
---
Nitpick comments:
In `@tests/ТестыЗамеров.os`:
- Around line 18-25: Перед вычислением Коэффициент убедитесь, что ВыделеноБайт
(из Результат.Запуски[0].Статистика.ВыделяемаяПамять) не равен нулю: добавьте
явную проверку (например, Ожидаем.Что(ВыделеноБайт).Больше(0) или эквивалентное
утверждение) сразу после получения ВыделеноБайт и только затем вычисляйте
Коэффициент = РазмерВБайтах / ВыделеноБайт; это предотвратит деление на ноль и
сделает причину падения теста очевидной.
In `@tests/ТестыИсполнителяБенчмарков.os`:
- Around line 118-121: The method chain starting at Конфигурация has
inconsistent indentation: add a leading tab before
`.УстановитьСтратегию(СтратегииЗапускаБенчмарка.ХолодныйЗапуск)` so the
`.УстановитьСтратегию`, `.УстановитьКоличествоИтераций(1)` and
`.ДобавитьМониторингПамяти()` calls align with the existing chained call style
used elsewhere; update the indentation for those chained methods to match the
pattern used on lines 79–81.
In `@tests/ТестыСценарияЗапускаБенчмарка.os`:
- Around line 25-30: The test creates separate КоллекцияДескрипторовБенчмарков
instances for the СценарийЗапускаБенчмарка and for each helper
КейсПустогоБенчмарка, so cases are not tied to the scenario's descriptor
collection; fix it by creating a single
КоллекцияДескрипторовБенчмарков/ДескрипторБенчмарка instance and passing that
same descriptor collection or descriptor into both the СценарийЗапускаБенчмарка
constructor and into the helper that builds КейсБенчмарка (or change
КейсПустогоБенчмарка to accept a descriptor parameter), and update the other
test locations (lines ~45-50, 66-73, 96-100) to reuse the same descriptor
instance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b728d90f-68cb-4145-a994-ee2538b012c2
📒 Files selected for processing (17)
src/BenchmarkOneScript/core/running/Классы/КейсБенчмарка.ostests/fixtures/АппендерЛога.ostests/ТестыБенчмарков.ostests/ТестыГенератораКейсовБенчмарков.ostests/ТестыЗамеров.ostests/ТестыЗапускателяФайловБенчмарков.ostests/ТестыИсполнителяБенчмарков.ostests/ТестыКонфигурации.ostests/ТестыМенеджераПроцессовБенчмарков.ostests/ТестыПарсераПараметровБенчмарковCLI.ostests/ТестыПодготовителяФайлаБенчмарка.ostests/ТестыПредставленияПоказателей.ostests/ТестыСериализаторНастроекЗапускаБенчмарков.ostests/ТестыСериализаторРезультатовБенчмарков.ostests/ТестыСтатистики.ostests/ТестыСценарияЗапускаБенчмарка.ostests/ТестыЭкспортеров.os
Comment on lines
+11
to
+20
| &Инициализация | ||
| Процедура ПередВсеми() Экспорт | ||
| _МенеджерВременныхФайлов = Новый МенеджерВременныхФайлов(); | ||
| _ПодготовительФайлаБенчмарка = Новый ПодготовительФайлаБенчмарка(_МенеджерВременныхФайлов); | ||
| КонецПроцедуры | ||
|
|
||
| &После | ||
| Процедура ПослеЗапускаТеста() Экспорт | ||
| &Завершение | ||
| Процедура ПослеВсех() Экспорт | ||
|
|
||
| МенеджерВременныхФайлов.Удалить(); | ||
| _МенеджерВременныхФайлов.Удалить(); |
There was a problem hiding this comment.
Изолируйте проверки количества временных файлов от состояния модуля.
_МенеджерВременныхФайлов живет до ПослеВсех(), поэтому ИмеетДлину(0) и ИмеетДлину(1) начинают зависеть от порядка выполнения и от того, сколько файлов уже накопил модуль. Здесь безопаснее проверять дельту внутри самого теста или сам ПодготовленныйФайл, а не абсолютную длину общего списка.
💡 Вариант с проверкой дельты
// Подготовка
ПутьКФайлу = "./tests/fixtures/benchmarks/Классы/ПустойБенчмарк.os";
+ КоличествоФайловДо = _МенеджерВременныхФайлов.Файлы().Количество();
// Действие
ПодготовленныйФайл = _ПодготовительФайлаБенчмарка.Подготовить(ПутьКФайлу);
// Проверка
Ожидаем.Что(ПодготовленныйФайл).Равно(ПутьКФайлу);
- Ожидаем.Что(_МенеджерВременныхФайлов.Файлы()).ИмеетДлину(0);
+ Ожидаем.Что(_МенеджерВременныхФайлов.Файлы()).ИмеетДлину(КоличествоФайловДо); // Подготовка
ПутьКФайлу = "./tests/fixtures/АлгоритмБезАннотации.os";
+ КоличествоФайловДо = _МенеджерВременныхФайлов.Файлы().Количество();
// Действие
ПодготовленныйФайл = _ПодготовительФайлаБенчмарка.Подготовить(ПутьКФайлу);
// Проверка
Ожидаем.Что(ФС.ФайлСуществует(ПодготовленныйФайл)).Равно(Истина);
- Ожидаем.Что(_МенеджерВременныхФайлов.Файлы()).ИмеетДлину(1);
+ Ожидаем.Что(_МенеджерВременныхФайлов.Файлы()).ИмеетДлину(КоличествоФайловДо + 1);Also applies to: 31-35, 46-55
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/ТестыПодготовителяФайлаБенчмарка.os` around lines 11 - 20, Текущие
проверки ИмеетДлину(0)/ИмеетДлину(1) зависят от глобального состояния модуля
(_МенеджерВременныхФайлов), потому что менеджер создаётся в ПередВсеми() и живёт
до ПослеВсех(); исправьте тесты, чтобы проверять дельту созданных временных
файлов вместо абсолютной длины: в тесте захватите текущее количество файлов из
_МенеджерВременныхФайлов перед вызовом методов ПодготовительФайлаБенчмарка,
выполните операцию и затем сравните разницу (новая длина минус старая) с
ожидаемым значением; альтернативно, адаптируйте
ПодготовленныйФайл/ПодготовительФайлаБенчмарка чтобы возвращал количество или
список созданных временных файлов и проверяйте это значение напрямую (исходные
точки: функции/процедуры _МенеджерВременныхФайлов, ПодготовительФайлаБенчмарка,
и места проверок ИмеетДлину в тестах).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
tests/ТестыПредставленияПоказателей.os (2)
150-150: Унифицируйте имя теста с остальными (ФорматироватьvsФормировать).На Line 150 имя
ТестДолжен_ФормироватьФиксированнуюСтрокувыбивается из общего шаблона именования в файле.✏️ Вариант переименования
-Процедура ТестДолжен_ФормироватьФиксированнуюСтроку() Экспорт +Процедура ТестДолжен_ФорматироватьФиксированнуюСтроку() Экспорт🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыПредставленияПоказателей.os` at line 150, Переименуйте процедуру ТестДолжен_ФормироватьФиксированнуюСтроку в соответствии с остальными тестами, например в ТестДолжен_ФорматироватьФиксированнуюСтроку; обновите все ссылки на эту процедуру (вызовы, регистрация в тест-раннере или комментарии) чтобы сохранить консистентное именование и избежать ошибок при запуске тестов.
93-103: Вынесите расчётРезультатСМинусомвнутрь условия.На Line 93-96 вычисление выполняется даже для
0, хотя проверка пропускается. Лучше слегка упростить тест.♻️ Небольшое упрощение
- РезультатСМинусом = ПредставленияПоказателейБенчмарков.ПредставлениеЧисла( - -ЧислоДляПроверки, - ЕдиницаИзмерения - ); - // Проверка Ожидаем.Что(Результат).Равно(ОжидаемоеПредставление); Если ЧислоДляПроверки <> 0 Тогда + РезультатСМинусом = ПредставленияПоказателейБенчмарков.ПредставлениеЧисла( + -ЧислоДляПроверки, + ЕдиницаИзмерения + ); Ожидаем.Что(РезультатСМинусом).Равно("-" + ОжидаемоеПредставление); КонецЕсли;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыПредставленияПоказателей.os` around lines 93 - 103, Расчёт РезультатСМинусом сейчас выполняется всегда (вызов ПредставленияПоказателейБенчмарков.ПредставлениеЧисла с -ЧислоДляПроверки), хотя проверка использует только ненулевые ЧислоДляПроверки; переместите вычисление РезультатСМинусом внутрь блока Если ЧислоДляПроверки <> 0 Тогда и используйте там ПредставленияПоказателейБенчмарков.ПредставлениеЧисла(-ЧислоДляПроверки, ЕдиницаИзмерения), оставив существующую проверку Ожидаем.Что(РезультатСМинусом).Равно("-" + ОжидаемоеПредставление); таким образом вычисление пропустится при нуле и тест станет проще.tests/ТестыИсточниковПараметров.os (1)
89-94: Свести дублирование инициализации конфигурации в один helper.В двух новых тестах повторяется одинаковая подготовка типа/конфигурации. Лучше вынести в служебную функцию, чтобы упростить поддержку и снизить риск расхождений при будущих изменениях.
♻️ Proposed refactor
+Функция ПолучитьКонфигурациюСАннотациями() Экспорт + ТипБенчмарка = Тип("БенчмаркСАннотациямиКонфигурации"); + Возврат Новый КонфигурацияБенчмарков(ТипБенчмарка); +КонецФункции + Процедура ТестДолжен_УстановитьИсточникПараметровВКонфигурации() Экспорт // Подготовка - Тип = Тип("БенчмаркСАннотациямиКонфигурации"); - - Конфигурация = Новый КонфигурацияБенчмарков(Тип) + Конфигурация = ПолучитьКонфигурациюСАннотациями() .ОчиститьИсточникиПараметров(); @@ Процедура ТестДолжен_ОчиститьИсточникиПараметровВКонфигурации() Экспорт // Подготовка - Тип = Тип("БенчмаркСАннотациямиКонфигурации"); - - Конфигурация = Новый КонфигурацияБенчмарков(Тип) + Конфигурация = ПолучитьКонфигурациюСАннотациями() .ДобавитьИсточникПараметров("ОбщееЗначение", "Получить");Also applies to: 110-115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыИсточниковПараметров.os` around lines 89 - 94, Tests duplicate the same setup creating Тип("БенчмаркСАннотациямиКонфигурации") and initializing Конфигурация = Новый КонфигурацияБенчмарков(Тип).ОчиститьИсточникиПараметров(); — extract this into a shared helper function (e.g., CreateBenchmarkConfig или InitBenchmarkConfig) and replace the repeated blocks in both tests with a call to that helper; ensure the helper returns the initialized Конфигурация (and optionally Тип) and update tests to use the returned values so callers reference the same symbols (Тип, КонфигурацияБенчмарков, ОчиститьИсточникиПараметров).tests/ТестыВалидацииКонфигурацииБенчмарков.os (1)
11-23: Unused_МенеджерВременныхФайловcan be removed.The temp files manager is declared and initialized but never used in any test procedure in this module. All tests only validate
КонфигурацияБенчмарковwithout creating temporary files.🧹 Proposed cleanup
-#Использовать tempfiles `#Использовать` "helpers" `#Использовать` "fixtures/benchmarks" -Перем _МенеджерВременныхФайлов; // МенеджерВременныхФайлов - -#Область ОбработчикиСобытий - -&Инициализация -Процедура ПередВсеми() Экспорт - _МенеджерВременныхФайлов = Новый МенеджерВременныхФайлов(); -КонецПроцедуры - -&Завершение -Процедура ПослеВсех() Экспорт - _МенеджерВременныхФайлов.Удалить(); -КонецПроцедуры - -#КонецОбласти - `#Область` Тесты🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыВалидацииКонфигурацииБенчмарков.os` around lines 11 - 23, Remove the unused temporary files manager: delete the module-level declaration "_МенеджерВременныхФайлов" and the setup/teardown code that instantiates and calls Удалить() in ПередВсеми() and ПослеВсех(); specifically remove the "Новый МенеджерВременныхФайлов()" call in ПередВсеми and the "_МенеджерВременныхФайлов.Удалить()" call in ПослеВсех() (or alternatively, if you intend to use it later, wire it into tests that create temp files), leaving only the tests that validate КонфигурацияБенчмарков.tests/ТестыКолонокИЭкспортаКонфигурацииБенчмарков.os (1)
11-23: Unused_МенеджерВременныхФайлов.Same as in the validation tests module - temp files manager is not used in any test procedure.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыКолонокИЭкспортаКонфигурацииБенчмарков.os` around lines 11 - 23, Переменная и менеджер _МенеджерВременныхФайлов создаются в ПередВсеми и удаляются в ПослеВсех, но нигде не используются в тестах; удалите объявление Перем _МенеджерВременныхФайлов и вызовы в процедурах ПередВсеми/ПослеВсех (или, если менеджер действительно нужен, используйте его в тестах), обновив процедуры ПередВсеми и ПослеВсех (имена: ПередВсеми, ПослеВсех, класс: МенеджерВременныхФайлов) соответственно — либо полностью убрать код инициализации/удаления, либо подключить менеджер к логике тестов.tests/ТестыОбъединенияКонфигурацииБенчмарков.os (1)
11-23: Unused_МенеджерВременныхФайлов.The temp files manager is not used in any test procedure - all tests only manipulate in-memory configuration objects.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыОбъединенияКонфигурацииБенчмарков.os` around lines 11 - 23, Переменная и вызовы менеджера временных файлов не используются; удалите объявление "_МенеджерВременныхФайлов", удалите или закомментируйте процедуры "&Инициализация Процедура ПередВсеми() Экспорт" и "&Завершение Процедура ПослеВсех() Экспорт" вместе с созданием и вызовом _МенеджерВременныхФайлов = Новый МенеджерВременныхФайлов() и _МенеджерВременныхФайлов.Удалить(), либо если нужен менеджер — фактически использовать экземпляр в тестах; проверяйте упоминания класса МенеджерВременныхФайлов и функции ПередВсеми/ПослеВсех при правке, чтобы не оставить невостребованные ссылки.tests/ТестыОтчетаБенчмарков.os (1)
82-84: Consider adding skip reason for conditional test exclusion.Tests silently return when running on OneScript1. Consider using a framework-provided skip mechanism or logging to make test results clearer when these tests are bypassed.
Also applies to: 122-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыОтчетаБенчмарков.os` around lines 82 - 84, The conditional currently just returns when ТестированиеБенчмарков.ЭтоOneScript1() is true, which hides that the test was intentionally skipped; replace the silent return with the test framework's skip/ignore mechanism (or at minimum a clear log message) so test reports show a skip reason — locate the checks using ТестированиеБенчмарков.ЭтоOneScript1() (the blocks at the shown occurrence and the similar one around lines 122-124) and call the framework skip function or emit a descriptive log such as "Skipping benchmark tests on OneScript1" instead of plain Возврат.tests/ТестыОсновныхНастроекКонфигурацииБенчмарков.os (1)
11-23: Consider removing unused_МенеджерВременныхФайлов.The temporary files manager is initialized and cleaned up but never used by any test in this module. This appears to be dead code.
🧹 Remove unused code
-Перем _МенеджерВременныхФайлов; // МенеджерВременныхФайлов - `#Область` ОбработчикиСобытий -&Инициализация -Процедура ПередВсеми() Экспорт - _МенеджерВременныхФайлов = Новый МенеджерВременныхФайлов(); -КонецПроцедуры - -&Завершение -Процедура ПослеВсех() Экспорт - _МенеджерВременныхФайлов.Удалить(); -КонецПроцедуры - -#КонецОбласти🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыОсновныхНастроекКонфигурацииБенчмарков.os` around lines 11 - 23, The _МенеджерВременныхФайлов variable and its lifecycle handlers (ПередВсеми and ПослеВсех) are unused and should be removed; delete the declaration "Перем _МенеджерВременныхФайлов;", the initialization in procedure ПередВсеми (new МенеджерВременныхФайлов()) and the cleanup call in procedure ПослеВсех (._Удалить()), or alternatively wire actual usage into tests—prefer removing these three places to eliminate dead code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/ТестыКолонокИЭкспортаКонфигурацииБенчмарков.os`:
- Around line 227-234: Тест добавляет в переменную Процентили произвольное
значение (33) но затем проверяет только значения из
КолонкаОтчетаБенчмарковПроцентиль.ПроцентилиПоУмолчанию(), поэтому добавленный
процентиль игнорируется; исправьте цикл: итерацию нужно выполнять не по
ПроцентилиПоУмолчанию(), а по самой переменной Процентили (та же логика проверок
с КолонкиОтчетаБенчмарков.Процентиль(Процентиль),
ТаблицаОтчета.Колонки.Найти(ИмяКолонки) и ТаблицаОтчета[0][ИмяКолонки]).
Обновите цикл "Для Каждого Процентиль Из ..." чтобы использовать Процентили
вместо вызова ПроцентилиПоУмолчанию().
---
Nitpick comments:
In `@tests/ТестыВалидацииКонфигурацииБенчмарков.os`:
- Around line 11-23: Remove the unused temporary files manager: delete the
module-level declaration "_МенеджерВременныхФайлов" and the setup/teardown code
that instantiates and calls Удалить() in ПередВсеми() and ПослеВсех();
specifically remove the "Новый МенеджерВременныхФайлов()" call in ПередВсеми and
the "_МенеджерВременныхФайлов.Удалить()" call in ПослеВсех() (or alternatively,
if you intend to use it later, wire it into tests that create temp files),
leaving only the tests that validate КонфигурацияБенчмарков.
In `@tests/ТестыИсточниковПараметров.os`:
- Around line 89-94: Tests duplicate the same setup creating
Тип("БенчмаркСАннотациямиКонфигурации") and initializing Конфигурация = Новый
КонфигурацияБенчмарков(Тип).ОчиститьИсточникиПараметров(); — extract this into a
shared helper function (e.g., CreateBenchmarkConfig или InitBenchmarkConfig) and
replace the repeated blocks in both tests with a call to that helper; ensure the
helper returns the initialized Конфигурация (and optionally Тип) and update
tests to use the returned values so callers reference the same symbols (Тип,
КонфигурацияБенчмарков, ОчиститьИсточникиПараметров).
In `@tests/ТестыКолонокИЭкспортаКонфигурацииБенчмарков.os`:
- Around line 11-23: Переменная и менеджер _МенеджерВременныхФайлов создаются в
ПередВсеми и удаляются в ПослеВсех, но нигде не используются в тестах; удалите
объявление Перем _МенеджерВременныхФайлов и вызовы в процедурах
ПередВсеми/ПослеВсех (или, если менеджер действительно нужен, используйте его в
тестах), обновив процедуры ПередВсеми и ПослеВсех (имена: ПередВсеми, ПослеВсех,
класс: МенеджерВременныхФайлов) соответственно — либо полностью убрать код
инициализации/удаления, либо подключить менеджер к логике тестов.
In `@tests/ТестыОбъединенияКонфигурацииБенчмарков.os`:
- Around line 11-23: Переменная и вызовы менеджера временных файлов не
используются; удалите объявление "_МенеджерВременныхФайлов", удалите или
закомментируйте процедуры "&Инициализация Процедура ПередВсеми() Экспорт" и
"&Завершение Процедура ПослеВсех() Экспорт" вместе с созданием и вызовом
_МенеджерВременныхФайлов = Новый МенеджерВременныхФайлов() и
_МенеджерВременныхФайлов.Удалить(), либо если нужен менеджер — фактически
использовать экземпляр в тестах; проверяйте упоминания класса
МенеджерВременныхФайлов и функции ПередВсеми/ПослеВсех при правке, чтобы не
оставить невостребованные ссылки.
In `@tests/ТестыОсновныхНастроекКонфигурацииБенчмарков.os`:
- Around line 11-23: The _МенеджерВременныхФайлов variable and its lifecycle
handlers (ПередВсеми and ПослеВсех) are unused and should be removed; delete the
declaration "Перем _МенеджерВременныхФайлов;", the initialization in procedure
ПередВсеми (new МенеджерВременныхФайлов()) and the cleanup call in procedure
ПослеВсех (._Удалить()), or alternatively wire actual usage into tests—prefer
removing these three places to eliminate dead code.
In `@tests/ТестыОтчетаБенчмарков.os`:
- Around line 82-84: The conditional currently just returns when
ТестированиеБенчмарков.ЭтоOneScript1() is true, which hides that the test was
intentionally skipped; replace the silent return with the test framework's
skip/ignore mechanism (or at minimum a clear log message) so test reports show a
skip reason — locate the checks using ТестированиеБенчмарков.ЭтоOneScript1()
(the blocks at the shown occurrence and the similar one around lines 122-124)
and call the framework skip function or emit a descriptive log such as "Skipping
benchmark tests on OneScript1" instead of plain Возврат.
In `@tests/ТестыПредставленияПоказателей.os`:
- Line 150: Переименуйте процедуру ТестДолжен_ФормироватьФиксированнуюСтроку в
соответствии с остальными тестами, например в
ТестДолжен_ФорматироватьФиксированнуюСтроку; обновите все ссылки на эту
процедуру (вызовы, регистрация в тест-раннере или комментарии) чтобы сохранить
консистентное именование и избежать ошибок при запуске тестов.
- Around line 93-103: Расчёт РезультатСМинусом сейчас выполняется всегда (вызов
ПредставленияПоказателейБенчмарков.ПредставлениеЧисла с -ЧислоДляПроверки), хотя
проверка использует только ненулевые ЧислоДляПроверки; переместите вычисление
РезультатСМинусом внутрь блока Если ЧислоДляПроверки <> 0 Тогда и используйте
там ПредставленияПоказателейБенчмарков.ПредставлениеЧисла(-ЧислоДляПроверки,
ЕдиницаИзмерения), оставив существующую проверку
Ожидаем.Что(РезультатСМинусом).Равно("-" + ОжидаемоеПредставление); таким
образом вычисление пропустится при нуле и тест станет проще.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a53599cd-8577-444e-962a-48b090d1f5ee
📒 Files selected for processing (27)
tests/ТестыВалидацииКонфигурацииБенчмарков.ostests/ТестыГенератораКейсовБенчмарков.ostests/ТестыДескрипторовБенчмарков.ostests/ТестыЗамеров.ostests/ТестыЗапускаБенчмарков.ostests/ТестыЗапускателяФайловБенчмарков.ostests/ТестыИсполнителяБенчмарков.ostests/ТестыИсполняющихСредКонфигурацииБенчмарков.ostests/ТестыИсточниковПараметров.ostests/ТестыКолонокИЭкспортаКонфигурацииБенчмарков.ostests/ТестыКонфигурации.ostests/ТестыМенеджераПроцессовБенчмарков.ostests/ТестыОбработчиковСобытийБенчмарков.ostests/ТестыОбъединенияКонфигурацииБенчмарков.ostests/ТестыОсновныхНастроекКонфигурацииБенчмарков.ostests/ТестыОтчетаБенчмарков.ostests/ТестыПарсераПараметровБенчмарковCLI.ostests/ТестыПодготовителяФайлаБенчмарка.ostests/ТестыПодключенияБенчмарков.ostests/ТестыПредставленияПоказателей.ostests/ТестыСериализаторНастроекЗапускаБенчмарков.ostests/ТестыСериализаторРезультатовБенчмарков.ostests/ТестыСтатистики.ostests/ТестыСтратегииХолодныйЗапуск.ostests/ТестыСценарияЗапускаБенчмарка.ostests/ТестыЭкспортеров.ostests/ТестыЭталонов.os
💤 Files with no reviewable changes (1)
- tests/ТестыКонфигурации.os
✅ Files skipped from review due to trivial changes (3)
- tests/ТестыСтратегииХолодныйЗапуск.os
- tests/ТестыЭталонов.os
- tests/ТестыИсполняющихСредКонфигурацииБенчмарков.os
🚧 Files skipped from review as they are similar to previous changes (8)
- tests/ТестыСериализаторНастроекЗапускаБенчмарков.os
- tests/ТестыПарсераПараметровБенчмарковCLI.os
- tests/ТестыМенеджераПроцессовБенчмарков.os
- tests/ТестыЭкспортеров.os
- tests/ТестыИсполнителяБенчмарков.os
- tests/ТестыСериализаторРезультатовБенчмарков.os
- tests/ТестыГенератораКейсовБенчмарков.os
- tests/ТестыЗапускателяФайловБенчмарков.os
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
tests/ТестыПредставленияПоказателейБенчмарков.os (1)
149-150: Унифицируйте нейминг теста с остальнымиТестДолжен_Форматировать*.На Line 150 используется
ТестДолжен_ФормироватьФиксированнуюСтроку; для консистентности лучшеТестДолжен_ФорматироватьФиксированнуюСтроку.Предлагаемый минимальный diff
-Процедура ТестДолжен_ФормироватьФиксированнуюСтроку() Экспорт +Процедура ТестДолжен_ФорматироватьФиксированнуюСтроку() Экспорт🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыПредставленияПоказателейБенчмарков.os` around lines 149 - 150, Rename the test procedure to match existing naming convention: change the exported procedure declaration and any references from "ТестДолжен_ФормироватьФиксированнуюСтроку" to "ТестДолжен_ФорматироватьФиксированнуюСтроку" (update the procedure header and any calls/registrations/export entries that reference the old name) so the test name is consistent with other "ТестДолжен_Форматировать*" tests.tests/ТестыСериализатораРезультатовБенчмарков.os (1)
155-161: Немного уточнить комментарии секций для лучшей читаемости.На Line 155 блок помечен как
// Действие, но фактически это извлечение данных перед проверками. Лучше переименовать, чтобы не сбивать при чтении.Предлагаемое уточнение комментариев
- // Действие + // Подготовка данных для проверки конфигурации // Конфигурация Конфигурация = РезультатПрочитанный.Конфигурация; ИсполняющиеСреды = Конфигурация.ИсполняющиеСреды(); - // Проверка + // Проверка конфигурации Ожидаем.Что(Конфигурация, "Конфигурация").ИмеетТип("КонфигурацияБенчмарков");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыСериализатораРезультатовБенчмарков.os` around lines 155 - 161, Поменяй метку секции комментария "// Действие" на более точную, например "// Подготовка" или "// Подготовка данных", поскольку в этом блоке выполняется извлечение Конфигурация из РезультатПрочитанный и инициализация ИсполняющиеСреды; обнови соседние секционные комментарии так же, чтобы они соответствовали фазам Arrange/Act/Assert (например, "// Проверка" оставить для вызова Ожидаем.Что и метода ИмеетТип), чтобы чтение теста с использованием переменных Конфигурация, РезультатПрочитанный, ИсполняющиеСреды и проверкой через Ожидаем.Что было однозначным.tests/ТестыИсточниковПараметровБенчмарков.os (2)
87-124: Добавьте тест на идемпотентностьДобавитьИсточникПараметров.Сейчас проверены только “добавить один” и “очистить”, но не проверяется поведение при повторном добавлении той же пары
(ИмяПоля, Источник). Это важный регрессионный кейс, т.к. метод де-дуплицирует записи.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыИсточниковПараметровБенчмарков.os` around lines 87 - 124, Добавьте тест проверяющий идемпотентность метода ДобавитьИсточникПараметров у класса КонфигурацияБенчмарков: создайте Конфигурация = Новый КонфигурацияБенчмарков(Тип).ОчиститьИсточникиПараметров(), вызовите дважды Конфигурация.ДобавитьИсточникПараметров("ОбщееЗначение","Получить"), затем получите ИсточникиПараметров = Конфигурация.ИсточникиПараметров() и assert'ы что длина равна 1 и что ИсточникиПараметров[0].ИмяПоля и .Источник равны ожидаемым; оставьте также существующие тесты ОчиститьИсточникиПараметров без изменений.
127-373: Снизьте дублирование setup-блока в тестах запуска.Во многих тестах повторяется одинаковая подготовка (
ПараметрыЗапуска/Конфигурация/Дескриптор). Вынесение в одну служебную функцию упростит поддержку и локальные правки.Пример направления рефакторинга
+Функция ПодготовитьСценарий(ИмяБенчмарка, ОчищатьИсточникиПолей = Истина) + ПараметрыЗапуска = ПолучитьПараметрыЗапуска(ОчищатьИсточникиПолей); + Конфигурация = ПараметрыЗапуска.Конфигурация; + Дескриптор = ПараметрыЗапуска.Дескрипторы.НайтиПоИмени(ИмяБенчмарка); + Возврат Новый Структура("Конфигурация, Дескриптор", Конфигурация, Дескриптор); +КонецФункции🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыИсточниковПараметровБенчмарков.os` around lines 127 - 373, В тестах повторяется подготовка: вызов ПолучитьПараметрыЗапуска, извлечение Конфигурация и поиск Дескриптора; создайте вспомогательную процедуру (например ПолучитьСтандартныеПараметрыЗапуска или SetupПараметровЗапуска) которая вернёт структуру с ПараметрыЗапуска, Конфигурация и Дескриптор, и замените в тестах повторяющиеся блоки на один вызов этой процедуры (обновите тесты: ТестДолжен_ЗапуститьСИсточникомПараметровДляОбщегоПоля, ТестДолжен_ЗапуститьСНесколькимиИсточникамиПараметровДляОбщегоПоля, ТестДолжен_ЗапуститьСИсточникомПараметровДляОбщегоПоляИзСтруктуры и т.д.), сохранив семантику (передача параметров в Конфигурацию и последующие действия) и не меняя имён проверок/ожиданий.tests/ТестыОбъединенияКонфигурацийБенчмарков.os (1)
166-166: Избегайте захардкоженного дефолта6в проверке прогревов.На Line 166 тест привязан к текущему значению по умолчанию, поэтому станет ложнопадающим при легитимной смене дефолта в
КонфигурацияБенчмарков.💡 Предложение
// Проверка + ОжидаемоеКоличествоПрогревовПоУмолчанию = Новый КонфигурацияБенчмарков().КоличествоПрогревочныхИтераций(); + ИсточникиПараметров = КонфигурацияПриемник.ИсточникиПараметров(); ОбработчикиСобытияПередВсеми = КонфигурацияПриемник.ОбработчикиСобытия(СобытияБенчмарков.ПередВсеми); ОбработчикиСобытияПослеВсех = КонфигурацияПриемник.ОбработчикиСобытия(СобытияБенчмарков.ПослеВсех); @@ - Ожидаем.Что(КонфигурацияПриемник.КоличествоПрогревочныхИтераций(), "Количество прогревочных итераций").Равно(6); + Ожидаем.Что(КонфигурацияПриемник.КоличествоПрогревочныхИтераций(), "Количество прогревочных итераций").Равно(ОжидаемоеКоличествоПрогревовПоУмолчанию);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыОбъединенияКонфигурацийБенчмарков.os` at line 166, Не привязывайте тест к хардкодному значению 6: вместо сравнения КонфигурацияПриемник.КоличествоПрогревочныхИтераций() с литералом 6, получите ожидаемое значение из места, где хранится дефолт (например метод/поле в КонфигурацияБенчмарков — используйте что-то вроде КонфигурацияБенчмарков.КоличествоПрогревочныхИтерацийПоУмолчанию() или другой доступный accessor) и сравнивайте КонфигурацияПриемник.КоличествоПрогревочныхИтераций() с этим значением; если такого метода нет, замените на проверку, которая валидирует семантику (например >0) вместо фиксированного числа.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/ТестыЗамеровБенчмарков.os`:
- Around line 17-20: The benchmark configuration created via Новый
КонфигурацияБенчмарков(Тип) that calls .УстановитьКоличествоИтераций(.),
.УстановитьКоличествоПрогревочныхИтераций(.), and
.УстановитьКоличествоВызововЗаИтерацию(.) must also enable memory tracking
because tests later assert memory metrics (ВыделяемаяПамять / этап Память);
update both occurrences of that configuration (the blocks starting at the shown
snippet and the second block around lines 141–144) to call
.ДобавитьМониторингПамяти() on the same configuration chain so memory sampling
is actually collected before assertions run.
In `@tests/ТестыОбъединенияКонфигурацийБенчмарков.os`:
- Around line 21-23: Сделайте процедуру ПослеВсех безопасной при частичной
инициализации: в теле процедуры ПослеВсех проверяйте существование/непустоту
переменной _МенеджерВременныхФайлов и оборачивайте вызов
_МенеджерВременныхФайлов.Удалить() в защищённый блок (или условие), чтобы при
сбое в ПередВсеми() вызов очистки не падал и не маскировал первичную ошибку;
используйте те же имена процедур/переменных (ПослеВсех, ПередВсеми,
_МенеджерВременныхФайлов) для поиска и правки.
---
Nitpick comments:
In `@tests/ТестыИсточниковПараметровБенчмарков.os`:
- Around line 87-124: Добавьте тест проверяющий идемпотентность метода
ДобавитьИсточникПараметров у класса КонфигурацияБенчмарков: создайте
Конфигурация = Новый КонфигурацияБенчмарков(Тип).ОчиститьИсточникиПараметров(),
вызовите дважды
Конфигурация.ДобавитьИсточникПараметров("ОбщееЗначение","Получить"), затем
получите ИсточникиПараметров = Конфигурация.ИсточникиПараметров() и assert'ы что
длина равна 1 и что ИсточникиПараметров[0].ИмяПоля и .Источник равны ожидаемым;
оставьте также существующие тесты ОчиститьИсточникиПараметров без изменений.
- Around line 127-373: В тестах повторяется подготовка: вызов
ПолучитьПараметрыЗапуска, извлечение Конфигурация и поиск Дескриптора; создайте
вспомогательную процедуру (например ПолучитьСтандартныеПараметрыЗапуска или
SetupПараметровЗапуска) которая вернёт структуру с ПараметрыЗапуска,
Конфигурация и Дескриптор, и замените в тестах повторяющиеся блоки на один вызов
этой процедуры (обновите тесты:
ТестДолжен_ЗапуститьСИсточникомПараметровДляОбщегоПоля,
ТестДолжен_ЗапуститьСНесколькимиИсточникамиПараметровДляОбщегоПоля,
ТестДолжен_ЗапуститьСИсточникомПараметровДляОбщегоПоляИзСтруктуры и т.д.),
сохранив семантику (передача параметров в Конфигурацию и последующие действия) и
не меняя имён проверок/ожиданий.
In `@tests/ТестыОбъединенияКонфигурацийБенчмарков.os`:
- Line 166: Не привязывайте тест к хардкодному значению 6: вместо сравнения
КонфигурацияПриемник.КоличествоПрогревочныхИтераций() с литералом 6, получите
ожидаемое значение из места, где хранится дефолт (например метод/поле в
КонфигурацияБенчмарков — используйте что-то вроде
КонфигурацияБенчмарков.КоличествоПрогревочныхИтерацийПоУмолчанию() или другой
доступный accessor) и сравнивайте
КонфигурацияПриемник.КоличествоПрогревочныхИтераций() с этим значением; если
такого метода нет, замените на проверку, которая валидирует семантику (например
>0) вместо фиксированного числа.
In `@tests/ТестыПредставленияПоказателейБенчмарков.os`:
- Around line 149-150: Rename the test procedure to match existing naming
convention: change the exported procedure declaration and any references from
"ТестДолжен_ФормироватьФиксированнуюСтроку" to
"ТестДолжен_ФорматироватьФиксированнуюСтроку" (update the procedure header and
any calls/registrations/export entries that reference the old name) so the test
name is consistent with other "ТестДолжен_Форматировать*" tests.
In `@tests/ТестыСериализатораРезультатовБенчмарков.os`:
- Around line 155-161: Поменяй метку секции комментария "// Действие" на более
точную, например "// Подготовка" или "// Подготовка данных", поскольку в этом
блоке выполняется извлечение Конфигурация из РезультатПрочитанный и
инициализация ИсполняющиеСреды; обнови соседние секционные комментарии так же,
чтобы они соответствовали фазам Arrange/Act/Assert (например, "// Проверка"
оставить для вызова Ожидаем.Что и метода ИмеетТип), чтобы чтение теста с
использованием переменных Конфигурация, РезультатПрочитанный, ИсполняющиеСреды и
проверкой через Ожидаем.Что было однозначным.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 89471d3b-4919-4538-b675-bf1fc2e43b5a
📒 Files selected for processing (10)
tests/ТестыЗамеровБенчмарков.ostests/ТестыИсточниковПараметровБенчмарков.ostests/ТестыОбъединенияКонфигурацийБенчмарков.ostests/ТестыПредставленияПоказателейБенчмарков.ostests/ТестыСериализатораНастроекЗапускаБенчмарков.ostests/ТестыСериализатораРезультатовБенчмарков.ostests/ТестыСтатистикиБенчмарков.ostests/ТестыСтратегииХолодногоЗапуска.ostests/ТестыЭкспортаРезультатовБенчмарков.ostests/ТестыЭталоновБенчмарков.os
✅ Files skipped from review due to trivial changes (1)
- tests/ТестыСтратегииХолодногоЗапуска.os
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
tests/ТестыИсполнителяБенчмарков.os (2)
77-95: Можно вынести повторяющийся setup в helper для снижения дублирования.Блоки создания
Тип/Конфигурация/Исполнительповторяются; небольшой helper упростит поддержку тестов при будущих изменениях конфигурации.Also applies to: 116-133
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыИсполнителяБенчмарков.os` around lines 77 - 95, Повторяющийся setup в тесте (создание Тип, Конфигурация через КонфигурацияБенчмарков и вызов ТестированиеБенчмарков.НастроитьКонфигурациюПодТесты, а затем создание ИсполнительБенчмарков) нужно вынести в helper-функцию (например CreateBenchmarkRunner или PrepareBenchmarkWithCheckpoint) которая возвращает подготовленный ИсполнительБенчмарков и/или результат первого запуска; замените повторяющиеся блоки на вызовы этого helper и используйте методы УстановитьЧекпоинт и Запустить на возвращаемом исполнителе, чтобы убрать дублирование и упростить поддержку теста (применить аналогично для блока 116-133).
46-63: Сделайте число итераций явным для детерминизма теста checkpoint.Сейчас утверждение
Результат.Чекпоинт=Неопределенозависит от дефолтной конфигурации. Лучше явно зафиксировать 1 итерацию в этом тесте (Line 46), чтобы поведение не менялось при изменении дефолтов.💡 Предлагаемое изменение
- Конфигурация.УстановитьСтратегию(СтратегииЗапускаБенчмарка.ХолодныйЗапуск); + Конфигурация + .УстановитьСтратегию(СтратегииЗапускаБенчмарка.ХолодныйЗапуск) + .УстановитьКоличествоИтераций(1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыИсполнителяБенчмарков.os` around lines 46 - 63, Тест полагается на дефолтное число итераций, поэтому сделайте количество итераций явным: до создания ИсполнительБенчмарков или перед запуском вызова Исполнитель.Запустить установите в объекте Конфигурация конкретное значение итераций (например, установить свойство/параметр КоличествоИтераций = 1 или вызвать соответствующий метод конфигурации) так, чтобы НомерИтерации и ожидаемое поведение Чекпоинта детерминированно соответствовали 1 итерации при проверке Результат.Чекпоинт; убедитесь, что изменение производится рядом с существующими строками, где используется Конфигурация и УстановитьСтратегию.tests/ТестыОсновныхНастроекКонфигурацииБенчмарков.os (3)
117-119: Добавьте поведенческие проверки, а не только echo-проверку конфигурации.В этих тестах в основном проверяется, что значение осталось в
Результат.Конфигурация; это может пройти даже если движок не применяет настройки к выполнению. Стоит дополнить проверками поРезультат.Запуски[0].Замеры(фактическое влияние на запуск).Also applies to: 138-140, 159-163
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыОсновныхНастроекКонфигурацииБенчмарков.os` around lines 117 - 119, Текущие проверки утверждают только, что значение сохранено в Результат.Конфигурация (например, Проверки вокруг Результат.Конфигурация.КоличествоПрогревочныхИтераций()), но не подтверждают, что настройка реально влияла на выполнение; добавьте поведенческие ассерт-правила, которые проверяют фактические замеры в Результат.Запуски[0].Замеры (например, длину массива замеров, наличие N прогревочных итераций или изменение времени/показателей до/после прогрева) чтобы убедиться, что КоличествоПрогревочныхИтераций применено при запуске; повторите аналогичную правку для блоков, указанных в комментарии (строки 138-140 и 159-163).
39-41: Уберите зависимость от порядка строк в отчете.Сейчас проверки привязаны к индексам строк, из‑за чего тесты могут стать flaky при изменении сортировки/порядка формирования отчета. Лучше проверять множество значений параметра, а не позиции.
🔧 Вариант правки (order-independent assertions)
- Ожидаем.Что(Результат.Отчет.Таблица[0][КолонкиОтчетаБенчмарков.Параметр("Значение")]).Равно(10); - Ожидаем.Что(Результат.Отчет.Таблица[1][КолонкиОтчетаБенчмарков.Параметр("Значение")]).Равно(100); + ЗначенияПараметра = Новый Массив; + Для Каждого СтрокаОтчета Из Результат.Отчет.Таблица Цикл + ЗначенияПараметра.Добавить(СтрокаОтчета[КолонкиОтчетаБенчмарков.Параметр("Значение")]); + КонецЦикла; + Ожидаем.Что(ЗначенияПараметра).Содержит(10); + Ожидаем.Что(ЗначенияПараметра).Содержит(100); + Ожидаем.Что(ЗначенияПараметра).ИмеетДлину(2);Also applies to: 62-64
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыОсновныхНастроекКонфигурацииБенчмарков.os` around lines 39 - 41, Текущие проверки привязаны к индексам строк в Результат.Отчет.Таблица (используют [0] и [1]), что делает тест флейким; замените их на независимую от порядка проверку: пройдите по всем строкам таблицы, соберите значения через КолонкиОтчетаБенчмарков.Парамет("Значение") в коллекцию и затем assert-ьте наличие ожидаемых элементов (10 и 100) либо сравните множества/многомножества с ожидаемым набором; используйте существующие проверки Ожидаем.Что для проверки "содержит" или "равно множеству" вместо обращения к конкретным индексам.
225-270: Разбейте большую helper-процедуру на тематические проверки.
ПроверитьКонфигурациюБенчмаркаСАннотациямиКонфигурацииделает слишком много сразу (стратегии, экспортеры, среды, колонки, параметры, источники). Декомпозиция упростит диагностику падений и сопровождение.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыОсновныхНастроекКонфигурацииБенчмарков.os` around lines 225 - 270, Текущая процедура ПроверитьКонфигурациюБенчмаркаСАннотациямиКонфигурации выполняет много разнородных проверок — разбей её на несколько тематических процедур/тестов: выделите отдельные процедуры/тесты ПроверитьСтратегиюИНастройки (проверки Конфигурация.Стратегия, СортировкаОтчета, ТребуетсяМониторингПамяти, КоличествоИтераций и т.д.), ПроверитьЭкспортеры (используя КоллекцияЭкспортеры и ФункцияПоискаЭкспортера), ПроверитьИсполняющиеСреды (проверки ИсполняющиеСреды[*].Версия/Наименование/ЭтоЭталон), ПроверитьКолонки (проверки Колонки содержит Мин/Макс), ПроверитьПараметры (проверки Параметры и их Значение) и ПроверитьИсточникиПараметров (проверка ИсточникиПараметров), оставив общий сбор данных (Экспортеры, КоллекцияЭкспортеры, ИсполняющиеСреды, Колонки, Параметры, ИсточникиПараметров) в небольшом setup-блоке; обновите вызовы в тестовой структуре так, чтобы каждый тематический чек вызывался отдельно от главной процедуры для лучшей изолированности и читаемости.tests/ТестыОбъединенияКонфигурацийБенчмарков.os (1)
146-148: Рассмотрите возможность добавить комментарий, поясняющий формулу расчёта прогревочных итераций.Значение
6в строке 148 предположительно рассчитывается автоматически на основеКоличествоИтераций = 222. Без комментария сложно понять, почему ожидается именно это значение, и тест станет хрупким при изменении формулы.💡 Предложение
+ // Примечание: КоличествоПрогревочныхИтераций рассчитывается автоматически + // на основе КоличествоИтераций по формуле: ceil(КоличествоИтераций * 0.027) или аналогичной Ожидаем.Что(КонфигурацияПриемник.КоличествоПрогревочныхИтераций(), "Количество прогревочных итераций").Равно(6);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыОбъединенияКонфигурацийБенчмарков.os` around lines 146 - 148, Добавьте поясняющий комментарий рядом с проверкой КонфигурацияПриемник.КоличествоПрогревочныхИтераций(), объясняющий точную формулу или правило, по которому рассчитывается ожидаемое значение 6 (например: "прогревочные итерации = max(1, round(КоличествоИтераций / N))" или другая используемая формула), укажите входные значения (КоличествоИтераций = 222) и источник правила (функция/конфиг/документация), и при необходимости добавьте ссылку на место в коде/конфиге где эта формула реализована (например имя метода/константы, отвечающей за расчёт) чтобы при изменении формулы тест было легко обновить.tests/ТестыКолонокИЭкспортаКонфигурацииБенчмарков.os (1)
1-269: Add lifecycle cleanup for benchmark artifacts to prevent inter-test contamination.The reviewed file has 8 tests that call
Бенчмаркинг.Запустить()without explicitly setting artifact directories, meaning they will create artifacts in the default location (.\BenchmarkArtifacts). Unlike other neighboring test modules (ТестыЭкспортаРезультатовБенчмарков.os,ТестыЗапускаБенчмарков.os,ТестыОтчетаБенчмарков.os, etc.), this file lacks an&После/ПослеЗапускаТеста()cleanup hook. This creates potential test pollution where artifacts from one test run remain and may affect subsequent test execution. Add the cleanup procedure:&После Процедура ПослеЗапускаТеста() Экспорт ФС.УдалитьФайлы(".\BenchmarkArtifacts"); КонецПроцедуры🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыКолонокИЭкспортаКонфигурацииБенчмарков.os` around lines 1 - 269, Add a lifecycle cleanup that removes the default artifact directory after each test run to avoid cross-test contamination: implement an &После exported procedure named ПослеЗапускаТеста that runs once after tests and calls the filesystem remover (e.g., invoke ФС.УдалитьФайлы on ".\BenchmarkArtifacts"); place it in this test file alongside existing tests so runs that call Бенчмаркинг.Запустить() will not leave artifacts behind.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/ТестыКолонокИЭкспортаКонфигурацииБенчмарков.os`:
- Around line 65-86: Текущий тест (Процедура
ТестДолжен_НеДублироватьКолонкуПриПовторномДобавлении) лишь проверяет наличие
колонки и значение, но не что она уникальна; добавьте явную проверку что имя
колонки встречается ровно один раз в результатной структуре столбцов: пройдитесь
по ТаблицаОтчета.Колонки (или используйте доступный метод поиска всех
совпадений) и посчитайте вхождения ИмяКолонки, затем добавьте утверждение
Ожидаем.Что(КоличествоВхождений).Равно(1); это изменение относится к проверке
после Результат = Бенчмаркинг.Запустить(Тип, Конфигурация) и затрагивает логику
вокруг КонфигурацияБенчмарков.ДобавитьКолонку и проверки на
ТаблицаОтчета.Колонки.Найти.
---
Nitpick comments:
In `@tests/ТестыИсполнителяБенчмарков.os`:
- Around line 77-95: Повторяющийся setup в тесте (создание Тип, Конфигурация
через КонфигурацияБенчмарков и вызов
ТестированиеБенчмарков.НастроитьКонфигурациюПодТесты, а затем создание
ИсполнительБенчмарков) нужно вынести в helper-функцию (например
CreateBenchmarkRunner или PrepareBenchmarkWithCheckpoint) которая возвращает
подготовленный ИсполнительБенчмарков и/или результат первого запуска; замените
повторяющиеся блоки на вызовы этого helper и используйте методы
УстановитьЧекпоинт и Запустить на возвращаемом исполнителе, чтобы убрать
дублирование и упростить поддержку теста (применить аналогично для блока
116-133).
- Around line 46-63: Тест полагается на дефолтное число итераций, поэтому
сделайте количество итераций явным: до создания ИсполнительБенчмарков или перед
запуском вызова Исполнитель.Запустить установите в объекте Конфигурация
конкретное значение итераций (например, установить свойство/параметр
КоличествоИтераций = 1 или вызвать соответствующий метод конфигурации) так,
чтобы НомерИтерации и ожидаемое поведение Чекпоинта детерминированно
соответствовали 1 итерации при проверке Результат.Чекпоинт; убедитесь, что
изменение производится рядом с существующими строками, где используется
Конфигурация и УстановитьСтратегию.
In `@tests/ТестыКолонокИЭкспортаКонфигурацииБенчмарков.os`:
- Around line 1-269: Add a lifecycle cleanup that removes the default artifact
directory after each test run to avoid cross-test contamination: implement an
&После exported procedure named ПослеЗапускаТеста that runs once after tests and
calls the filesystem remover (e.g., invoke ФС.УдалитьФайлы on
".\BenchmarkArtifacts"); place it in this test file alongside existing tests so
runs that call Бенчмаркинг.Запустить() will not leave artifacts behind.
In `@tests/ТестыОбъединенияКонфигурацийБенчмарков.os`:
- Around line 146-148: Добавьте поясняющий комментарий рядом с проверкой
КонфигурацияПриемник.КоличествоПрогревочныхИтераций(), объясняющий точную
формулу или правило, по которому рассчитывается ожидаемое значение 6 (например:
"прогревочные итерации = max(1, round(КоличествоИтераций / N))" или другая
используемая формула), укажите входные значения (КоличествоИтераций = 222) и
источник правила (функция/конфиг/документация), и при необходимости добавьте
ссылку на место в коде/конфиге где эта формула реализована (например имя
метода/константы, отвечающей за расчёт) чтобы при изменении формулы тест было
легко обновить.
In `@tests/ТестыОсновныхНастроекКонфигурацииБенчмарков.os`:
- Around line 117-119: Текущие проверки утверждают только, что значение
сохранено в Результат.Конфигурация (например, Проверки вокруг
Результат.Конфигурация.КоличествоПрогревочныхИтераций()), но не подтверждают,
что настройка реально влияла на выполнение; добавьте поведенческие
ассерт-правила, которые проверяют фактические замеры в
Результат.Запуски[0].Замеры (например, длину массива замеров, наличие N
прогревочных итераций или изменение времени/показателей до/после прогрева) чтобы
убедиться, что КоличествоПрогревочныхИтераций применено при запуске; повторите
аналогичную правку для блоков, указанных в комментарии (строки 138-140 и
159-163).
- Around line 39-41: Текущие проверки привязаны к индексам строк в
Результат.Отчет.Таблица (используют [0] и [1]), что делает тест флейким;
замените их на независимую от порядка проверку: пройдите по всем строкам
таблицы, соберите значения через КолонкиОтчетаБенчмарков.Парамет("Значение") в
коллекцию и затем assert-ьте наличие ожидаемых элементов (10 и 100) либо
сравните множества/многомножества с ожидаемым набором; используйте существующие
проверки Ожидаем.Что для проверки "содержит" или "равно множеству" вместо
обращения к конкретным индексам.
- Around line 225-270: Текущая процедура
ПроверитьКонфигурациюБенчмаркаСАннотациямиКонфигурации выполняет много
разнородных проверок — разбей её на несколько тематических процедур/тестов:
выделите отдельные процедуры/тесты ПроверитьСтратегиюИНастройки (проверки
Конфигурация.Стратегия, СортировкаОтчета, ТребуетсяМониторингПамяти,
КоличествоИтераций и т.д.), ПроверитьЭкспортеры (используя КоллекцияЭкспортеры и
ФункцияПоискаЭкспортера), ПроверитьИсполняющиеСреды (проверки
ИсполняющиеСреды[*].Версия/Наименование/ЭтоЭталон), ПроверитьКолонки (проверки
Колонки содержит Мин/Макс), ПроверитьПараметры (проверки Параметры и их
Значение) и ПроверитьИсточникиПараметров (проверка ИсточникиПараметров), оставив
общий сбор данных (Экспортеры, КоллекцияЭкспортеры, ИсполняющиеСреды, Колонки,
Параметры, ИсточникиПараметров) в небольшом setup-блоке; обновите вызовы в
тестовой структуре так, чтобы каждый тематический чек вызывался отдельно от
главной процедуры для лучшей изолированности и читаемости.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8186ef31-a988-494f-8bb9-ee3015853fad
📒 Files selected for processing (13)
.bsl-language-server.jsonsrc/BenchmarkOneScript/core/running/Классы/КейсБенчмарка.ostests/ТестыВалидацииКонфигурацииБенчмарков.ostests/ТестыГенератораКейсовБенчмарков.ostests/ТестыЗамеровБенчмарков.ostests/ТестыИсполнителяБенчмарков.ostests/ТестыКолонокИЭкспортаКонфигурацииБенчмарков.ostests/ТестыМенеджераПроцессовБенчмарков.ostests/ТестыОбъединенияКонфигурацийБенчмарков.ostests/ТестыОсновныхНастроекКонфигурацииБенчмарков.ostests/ТестыПредставленияПоказателейБенчмарков.ostests/ТестыСценарияЗапускаБенчмарка.ostests/ТестыЭкспортаРезультатовБенчмарков.os
✅ Files skipped from review due to trivial changes (2)
- tests/ТестыГенератораКейсовБенчмарков.os
- tests/ТестыСценарияЗапускаБенчмарка.os
🚧 Files skipped from review as they are similar to previous changes (5)
- src/BenchmarkOneScript/core/running/Классы/КейсБенчмарка.os
- tests/ТестыВалидацииКонфигурацииБенчмарков.os
- tests/ТестыЭкспортаРезультатовБенчмарков.os
- tests/ТестыМенеджераПроцессовБенчмарков.os
- tests/ТестыПредставленияПоказателейБенчмарков.os
Comment on lines
+65
to
+86
| Процедура ТестДолжен_НеДублироватьКолонкуПриПовторномДобавлении() Экспорт | ||
|
|
||
| // Подготовка | ||
| Тип = Тип("ПустойБенчмарк"); | ||
|
|
||
| ИмяКолонки = КолонкиОтчетаБенчмарков.Мин; | ||
|
|
||
| Конфигурация = Новый КонфигурацияБенчмарков(Тип); | ||
| Конфигурация.ДобавитьКолонку(ИмяКолонки); | ||
| Конфигурация.ДобавитьКолонку(ИмяКолонки); | ||
|
|
||
| ТестированиеБенчмарков.НастроитьКонфигурациюПодТесты(Конфигурация); | ||
|
|
||
| // Действие | ||
| Результат = Бенчмаркинг.Запустить(Тип, Конфигурация); | ||
|
|
||
| // Проверка | ||
| ТаблицаОтчета = Результат.Отчет.Таблица; | ||
|
|
||
| Ожидаем.Что(ТаблицаОтчета.Колонки.Найти(ИмяКолонки)).Не_().ЭтоНеопределено(); | ||
| Ожидаем.Что(ТаблицаОтчета[0][ИмяКолонки]).Заполнено(); | ||
|
|
There was a problem hiding this comment.
Тест недублирования не подтверждает отсутствие дубликата.
Сейчас проверяется только факт наличия колонки и значения. Если дубль попадёт в конфигурацию, этот тест может остаться зелёным. Добавьте явную проверку, что колонка встречается ровно один раз.
💡 Proposed fix
Ожидаем.Что(ТаблицаОтчета.Колонки.Найти(ИмяКолонки)).Не_().ЭтоНеопределено();
+ КоличествоКолонокСИменем = 0;
+ Для Каждого КолонкаТаблицы Из ТаблицаОтчета.Колонки Цикл
+ Если КолонкаТаблицы.Имя = ИмяКолонки Тогда
+ КоличествоКолонокСИменем = КоличествоКолонокСИменем + 1;
+ КонецЕсли;
+ КонецЦикла;
+ Ожидаем.Что(КоличествоКолонокСИменем).Равно(1);
Ожидаем.Что(ТаблицаОтчета[0][ИмяКолонки]).Заполнено();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/ТестыКолонокИЭкспортаКонфигурацииБенчмарков.os` around lines 65 - 86,
Текущий тест (Процедура ТестДолжен_НеДублироватьКолонкуПриПовторномДобавлении)
лишь проверяет наличие колонки и значение, но не что она уникальна; добавьте
явную проверку что имя колонки встречается ровно один раз в результатной
структуре столбцов: пройдитесь по ТаблицаОтчета.Колонки (или используйте
доступный метод поиска всех совпадений) и посчитайте вхождения ИмяКолонки, затем
добавьте утверждение Ожидаем.Что(КоличествоВхождений).Равно(1); это изменение
относится к проверке после Результат = Бенчмаркинг.Запустить(Тип, Конфигурация)
и затрагивает логику вокруг КонфигурацияБенчмарков.ДобавитьКолонку и проверки на
ТаблицаОтчета.Колонки.Найти.
|
0 New Issues
0 Fixed Issues
0 Accepted Issues
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Tests
Refactor