fix: Итерации бенчмарка стратегии "Холодный запуск" выполняются в отдельных процессах#40
Merged
fix: Итерации бенчмарка стратегии "Холодный запуск" выполняются в отдельных процессах#40
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughIntroduce isolated child-process benchmark execution with checkpointing and debug-port control; replace worker mode and old settings serializer with new launch serializer/DTOs; rename descriptor accessor to ТипКласса(); add process manager, scenario writer, exporters, resolver, checkpoint types, config manager, math helpers, and many test updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Main as Main Process
participant Launcher as ИзолированныйЗапускательБенчмарков
participant Validator as ВалидаторЗапускаБенчмарков
participant ProcMgr as МенеджерПроцессовБенчмарков
participant Child as Child Process
participant Exporter as ЭкспортерРезультатовБенчмарков
Main->>Launcher: ПриСозданииОбъекта(Источник, Конфигурация)
Main->>Launcher: УстановитьПортОтладки(Порт)
Main->>Launcher: ЗапуститьБенчмарки()
Launcher->>Validator: Проверить запуск (дескрипторы)
Validator-->>Launcher: Результат проверки
loop per environment
Launcher->>ProcMgr: ЗапуститьПроцесс(ИсполняющаяСреда, Чекпоинт)
ProcMgr->>ProcMgr: Сгенерировать сценарий и settings.json
ProcMgr->>Child: Старт (путь настроек, порт)
Child->>Child: Выполнить бенчмарки и записать JSON-результат
Child-->>ProcMgr: Завершение + путь результата
ProcMgr->>Launcher: Прочитать и вернуть результат
Launcher->>Launcher: Агрегировать результаты/чекпоинты
end
Launcher->>Exporter: Записать(АгрегированныеРезультаты, Каталог)
Exporter-->>Main: Список артефактов / итог
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: 10
🧹 Nitpick comments (4)
src/BenchmarkOneScript/core/Классы/МенеджерПараметровКонфигурацииOneScript.os (1)
68-71: Сделайте сериализациюoscript.cfgдетерминированной.На Lines 68-71 используется прямой обход
Соответствие, из-за чего порядок строк может плавать между запусками. Для стабильного вывода лучше сортировать ключи перед записью.💡 Вариант рефакторинга
МассивСтрок = Новый Массив(); - Для Каждого Строка Из ПрочитанныеНастройки Цикл - МассивСтрок.Добавить(СтрШаблон("%1=%2", Строка.Ключ, Строка.Значение)); + Ключи = Новый Массив(); + Для Каждого Элемент Из ПрочитанныеНастройки Цикл + Ключи.Добавить(Элемент.Ключ); + КонецЦикла; + Ключи.Сортировать(); + Для Каждого Ключ Из Ключи Цикл + МассивСтрок.Добавить(СтрШаблон("%1=%2", Ключ, ПрочитанныеНастройки.Получить(Ключ))); КонецЦикла;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/Классы/МенеджерПараметровКонфигурацииOneScript.os` around lines 68 - 71, Сериализация oscript.cfg нестабильна из‑за обхода Соответствия в произвольном порядке; в методе в классе МенеджерПараметровКонфигурацииOneScript (участок с ПрочитанныеНастройки, МассивСтрок и добавлением строк через СтрШаблон("%1=%2", Строка.Ключ, Строка.Значение)) соберите все ключи из ПрочитанныеНастройки в массив, отсортируйте этот массив ключей и затем итерируйтесь по отсортированному списку, извлекая значения по ключу и добавляя строки в МассивСтрок — это обеспечит детерминированный порядок при записи.docs/CLI.md (1)
27-27: Стоит добавить пример значения для--debugв таблице.Сейчас колонка “Пример” пустая;
--debug 9229сделает поведение опции очевиднее.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/CLI.md` at line 27, Добавьте пример значения для опции в строке таблицы, обновив ячейку "Пример" для `--debug` (строка с `| --debug | Порт отладки | |`) чтобы показать конкретное значение, например `--debug 9229`, так что строка станет `| --debug | Порт отладки | --debug 9229 |`, чтобы пользователям было сразу понятно ожидаемое значение.src/BenchmarkOneScript/core/Классы/ЗапускательБенчмарков.os (1)
408-434: Распаковку делегата стоит вынести в общий helper.В
ЗамеритьВремя()иЗамеритьПамять()повторяется один и тот же блок извлеченияОбъект/ИмяМетода/Параметры.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/Классы/ЗапускательБенчмарков.os` around lines 408 - 434, В обоих методах ЗамеритьВремя и ЗамеритьПамять повторяется код извлечения частей делегата (Рефлектор, Объект, ИмяМетода, Параметры); вынеси эту логику в общий helper-функцию (например, РаспаковатьДелегат или ПолучитьЧастиДелегата) которая принимает Делегат и возвращает структуру или набор значений {Рефлектор, Объект, ИмяМетода, Параметры}, затем используй этот helper внутри ЗамеритьВремя и ЗамеритьПамять вместо дублированного блока, сохрани семантику вызовов Рефлектор и имена переменных для минимальных изменений.src/BenchmarkOneScript/core/Классы/СценарийЗапускаБенчмарка.os (1)
42-42: Лучше не использовать одинаковое имя для переменной и функции (ТекстСценария).Это снижает читаемость и усложняет отладку, хотя код формально рабочий.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/Классы/СценарийЗапускаБенчмарка.os` at line 42, В текущем коде имя функции ТекстСценария и локальной переменной совпадает, что снижает читаемость и может вводить в заблуждение; переименуйте переменную (например в ТекстСценарияФайла или текстСценария) и замените все её последующие использования в классе/файле, оставив функцию ТекстСценария(ПутьФайлаНастроекЗапуска) без изменений, чтобы устранить затемнение имени и сохранить явное разделение функции и её результата.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/BenchmarkOneScript/cmd/Классы/КомандаЗапуска.os`:
- Around line 55-58: Validate the --debug port value stored in _ПортОтладки to
ensure it is within 1..65535 (not just >0) before passing it further; after
parsing the &Опция("debug") value (the variable _ПортОтладки) add a check that
if _ПортОтладки < 1 or > 65535 you emit a clear error and abort/throw (or set to
a safe default) so invalid ports cannot proceed into the pipeline; apply the
same range check to the other occurrence referenced at lines ~96-98.
In `@src/BenchmarkOneScript/core/Классы/ДескрипторБенчмарка.os`:
- Around line 277-279: В методе Скопировать() вы передаёте объект
ПараметрыМетодаБенчмарка в вызов ДобавитьПараметры(), тогда как
ДобавитьПараметры() ожидает массив значений (или одиночное значение). Измените
цикл по НаборыПараметров() так, чтобы вместо целого объекта передавалось либо
поле, содержащее массив значений (например Параметры.Значения или
Параметры.МассивЗначений), либо нужное одиночное значение (например
Параметры.Значение) — либо выполните явное преобразование объекта в массив
значений перед вызовом ДобавитьПараметры(). Убедитесь, что ссылку на
ПараметрыМетодаБенчмарка заменили на соответствующее поле/массив в каждом вызове
ДобавитьПараметры().
In `@src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os`:
- Around line 121-165: You are passing the same checkpoint into
ТребуетсяВыводСтатистики because Чекпоинт is overwritten before the call; save
the previous checkpoint and use it for the comparison: capture a
ПредыдущийЧекпоинт before assigning Чекпоинт = ТекущийРезультат.Чекпоинт (or
change ОбработатьИзолированныйЗапуск signature to accept both previous and
current), then inside ОбработатьИзолированныйЗапуск compute ТекущийЧекпоинт =
ТекущийРезультат.Чекпоинт and call ТребуетсяВыводСтатистики(ПредыдущийЧекпоинт,
ТекущийЧекпоинт) instead of comparing the same Чекпоинт values; reference
symbols: Чекпоинт, ОбработатьИзолированныйЗапуск, ТекущийРезультат,
ТребуетсяВыводСтатистики.
In `@src/BenchmarkOneScript/core/Классы/КонфигурацияБенчмарков.os`:
- Around line 925-929: При выполнении Объединить() текущая проверка на
ЗначенияМодифицированныхПолей[ИмяПоля_ТребуетсяМониторингПамяти()] = Истина
неверно сбрасывает мониторинг при значении Неопределено; исправьте условие так,
чтобы УдалитьМониторингПамяти() вызывался только когда значение явно = Ложь, а
ДобавитьМониторингПамяти() — когда явно = Истина; то есть заменить конструкцию
If ... Then ... Else ... EndIf на ветвление по явным значениям (If = Истина Then
... ElseIf = Ложь Then ... EndIf) используя массив
ЗначенияМодифицированныхПолей, функцию ИмяПоля_ТребуетсяМониторингПамяти() и
методы ДобавитьМониторингПамяти/УдалитьМониторингПамяти.
In
`@src/BenchmarkOneScript/core/Классы/МенеджерПараметровКонфигурацииOneScript.os`:
- Around line 100-114: Метод собирает настройки только из файлов и сразу
возвращает ПрочитанныеНастройки, но контракт функции Прочитать() обещает
дополнение системными переменными окружения; исправьте это, добавив после цикла
по КаталогиПоиска шаг с чтением и слиянием переменных окружения в
ПрочитанныеНастройки (используйте текущие имена переменных ПрочитанныеНастройки
и функции ПрочитатьКонфигурационныйФайл для поиска места), например пройти по
набору переменных окружения и записать их в ПрочитанныеНастройки, не
перезаписывая уже существующие ключи без явного намерения; затем вернуть
обновлённый ПрочитанныеНастройки.
- Around line 145-151: При разборе строки каталогов (Каталоги =
СтрРазделить(КаталогиСтрокой, ";")) пустые или состоящие из пробелов сегменты
сейчас передаются в цикл "Для Каждого Каталог Из Каталоги Цикл" и в функцию
ПолучитьАбсолютныйПуть, что приводит к добавлению лишних путей; исправьте это,
пропуская пустые/только-пробельные элементы: в теле цикла предварительно
выполнить обрезку/проверку (trim) переменной Каталог и продолжать цикл
(continue) если результат пустой, прежде чем вызывать ПолучитьАбсолютныйПуть и
добавлять в РазрешенныеКаталоги; оставьте остальную логику с использованием
КаталогРазрешенияПутей без изменений.
In `@src/BenchmarkOneScript/core/Классы/МенеджерПроцессовБенчмарков.os`:
- Around line 66-90: Поместите удаление временных файлов в блок окончательной
обработки: оберните код от создания/запуска процесса (включая вызовы
СоздатьИЗапуститьПроцесс, ОжидатьЗавершениеПроцесса,
ОбработатьВыполнениеПроцесса и ПрочитатьРезультатЗапускаБенчмарковИзJSON) в
конструкцию try/finally (в 1С — Попытка...Наконец...КонецПопытки) и вызовите
_МенеджерВременныхФайлов.Удалить() внутри раздела Наконец, чтобы гарантировать
удаление артефактов даже при возникновении исключений; не удаляйте логику
обработки ошибок, просто переместите вызов Удалить() в Наконец.
- Around line 280-282: В методе класса МенеджерПроцессовБенчмарков, где
проверяется наличие файла отчёта по переменной ПутьКОтчету, замените возврат
Неопределено на явную ошибку: вместо тихого возврата бросайте исключение или
возвращайте ошибочный результат с описанием (включая путь к файлу) и
логированием, чтобы верхний цикл мог корректно обработать провал; найдите блок с
Проверкой ФС.ФайлСуществует(ПутьКОтчету) и замените поведение на
генерацию/возврат ошибки с понятным сообщением об отсутствии JSON-отчёта.
In `@src/BenchmarkOneScript/core/Классы/СериализаторРезультатовБенчмарков.os`:
- Line 40: Вызов логера _Лог.Отладка использует некорректную формулировку "в
JSON-файла"; поправьте строку формата, заменив её на "Чтение результатов запуска
из JSON-файла: %1", оставив параметр ПолноеИмяФайла без изменений, чтобы
сообщение стало грамматически правильным.
In `@src/BenchmarkOneScript/core/Классы/ЭкспортерРезультатовБенчмарков.os`:
- Around line 9-10: Перед использованием
РезультатЗапуска.ДескрипторыБенчмарков.ПолучитьПервый().ТипКласса() добавьте
защиту на пустую коллекцию: проверьте ДескрипторыБенчмарков на пустоту (напр.,
ДескрипторыБенчмарков.Пуст() или Количество()=0) и в этом случае выберите
безопасное поведение — задать запасное имя файла/маску или корректно прекратить
экспорт с дружелюбной ошибкой; затем только в ветке с ненулевой коллекцией
формируйте ИмяФайла = СтрШаблон("%1-report",
Строка(РезультатЗапуска.ДескрипторыБенчмарков.ПолучитьПервый().ТипКласса())) и
МаскаФайлов = СтрШаблон("%1.*", ИмяФайла).
---
Nitpick comments:
In `@docs/CLI.md`:
- Line 27: Добавьте пример значения для опции в строке таблицы, обновив ячейку
"Пример" для `--debug` (строка с `| --debug | Порт отладки | |`) чтобы показать
конкретное значение, например `--debug 9229`, так что строка станет `| --debug |
Порт отладки | --debug 9229 |`, чтобы пользователям было сразу понятно ожидаемое
значение.
In `@src/BenchmarkOneScript/core/Классы/ЗапускательБенчмарков.os`:
- Around line 408-434: В обоих методах ЗамеритьВремя и ЗамеритьПамять
повторяется код извлечения частей делегата (Рефлектор, Объект, ИмяМетода,
Параметры); вынеси эту логику в общий helper-функцию (например,
РаспаковатьДелегат или ПолучитьЧастиДелегата) которая принимает Делегат и
возвращает структуру или набор значений {Рефлектор, Объект, ИмяМетода,
Параметры}, затем используй этот helper внутри ЗамеритьВремя и ЗамеритьПамять
вместо дублированного блока, сохрани семантику вызовов Рефлектор и имена
переменных для минимальных изменений.
In
`@src/BenchmarkOneScript/core/Классы/МенеджерПараметровКонфигурацииOneScript.os`:
- Around line 68-71: Сериализация oscript.cfg нестабильна из‑за обхода
Соответствия в произвольном порядке; в методе в классе
МенеджерПараметровКонфигурацииOneScript (участок с ПрочитанныеНастройки,
МассивСтрок и добавлением строк через СтрШаблон("%1=%2", Строка.Ключ,
Строка.Значение)) соберите все ключи из ПрочитанныеНастройки в массив,
отсортируйте этот массив ключей и затем итерируйтесь по отсортированному списку,
извлекая значения по ключу и добавляя строки в МассивСтрок — это обеспечит
детерминированный порядок при записи.
In `@src/BenchmarkOneScript/core/Классы/СценарийЗапускаБенчмарка.os`:
- Line 42: В текущем коде имя функции ТекстСценария и локальной переменной
совпадает, что снижает читаемость и может вводить в заблуждение; переименуйте
переменную (например в ТекстСценарияФайла или текстСценария) и замените все её
последующие использования в классе/файле, оставив функцию
ТекстСценария(ПутьФайлаНастроекЗапуска) без изменений, чтобы устранить
затемнение имени и сохранить явное разделение функции и её результата.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2e5cbeee-f08f-44c9-8e1b-db69ffc171d5
📒 Files selected for processing (34)
.bsl-language-server.jsondocs/CLI.mddocs/Эталоны.mdsamples/benchmarks/features/БенчмаркЕдиничныйВыброс.ossamples/benchmarks/features/БенчмаркСтратегияХолодныйЗапуск.ossrc/BenchmarkOneScript/cmd/Классы/КомандаЗапуска.ossrc/BenchmarkOneScript/core/Классы/ВалидаторЗапускаБенчмарков.ossrc/BenchmarkOneScript/core/Классы/ДескрипторБенчмарка.ossrc/BenchmarkOneScript/core/Классы/ЗапускательБенчмарков.ossrc/BenchmarkOneScript/core/Классы/ЗапускательБенчмарковИзКаталога.ossrc/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.ossrc/BenchmarkOneScript/core/Классы/КоллекцияДескрипторовБенчмарков.ossrc/BenchmarkOneScript/core/Классы/КонсольныйРепортерБенчмарков.ossrc/BenchmarkOneScript/core/Классы/КонфигурацияБенчмарков.ossrc/BenchmarkOneScript/core/Классы/МенеджерПараметровКонфигурацииOneScript.ossrc/BenchmarkOneScript/core/Классы/МенеджерПроцессовБенчмарков.ossrc/BenchmarkOneScript/core/Классы/НулевойРепортерБенчмарков.ossrc/BenchmarkOneScript/core/Классы/ОберткаИсточникаБенчмарков.ossrc/BenchmarkOneScript/core/Классы/РезультатЗапускаБенчмарков.ossrc/BenchmarkOneScript/core/Классы/СериализаторНастроекБенчмарков.ossrc/BenchmarkOneScript/core/Классы/СериализаторНастроекЗапускаБенчмарков.ossrc/BenchmarkOneScript/core/Классы/СериализаторРезультатовБенчмарков.ossrc/BenchmarkOneScript/core/Классы/СтатистикаБенчмарка.ossrc/BenchmarkOneScript/core/Классы/СценарийЗапускаБенчмарка.ossrc/BenchmarkOneScript/core/Классы/ЧекпоинтЗапускаБенчмарков.ossrc/BenchmarkOneScript/core/Классы/ЭкспортерРезультатовБенчмарков.ossrc/BenchmarkOneScript/core/Модули/Бенчмаркинг.ossrc/BenchmarkOneScript/core/Модули/МатематическиеФункцииБенчмарков.ossrc/BenchmarkOneScript/dto/Классы/НастройкиЗапускаБенчмарковДто.ossrc/BenchmarkOneScript/dto/Классы/РезультатЗапускаБенчмаркаДто.ostests/ТестСтратегииХолодныйЗапуск.ostests/ТестыСериализаторНастроекЗапускаБенчмарков.ostests/ТестыСериализаторРезультатовБенчмарков.ostests/ТестыСтатистики.os
💤 Files with no reviewable changes (1)
- src/BenchmarkOneScript/core/Классы/СериализаторНастроекБенчмарков.os
Comment on lines
+55
to
+58
| &Опция(Имя = "debug", Описание = "Порт отладки") | ||
| &ТЧисло | ||
| &ПоУмолчанию(0) | ||
| Перем _ПортОтладки; // Число |
There was a problem hiding this comment.
Добавьте валидацию диапазона порта для --debug.
Сейчас проверяется только > 0; значение за пределами 1..65535 уйдет дальше по пайплайну и может падать уже внутри запуска.
💡 Предлагаемый фикс
+ Если _ПортОтладки < 0 Или _ПортОтладки > 65535 Тогда
+ ВызватьИсключение СтрШаблон("Некорректный порт отладки: %1. Допустимый диапазон: 0..65535", _ПортОтладки);
+ КонецЕсли;
+
Для Каждого Тип Из ПодключенныеТипы ЦиклAlso applies to: 96-98
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/BenchmarkOneScript/cmd/Классы/КомандаЗапуска.os` around lines 55 - 58,
Validate the --debug port value stored in _ПортОтладки to ensure it is within
1..65535 (not just >0) before passing it further; after parsing the
&Опция("debug") value (the variable _ПортОтладки) add a check that if
_ПортОтладки < 1 or > 65535 you emit a clear error and abort/throw (or set to a
safe default) so invalid ports cannot proceed into the pipeline; apply the same
range check to the other occurrence referenced at lines ~96-98.
src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os
Show resolved
Hide resolved
Comment on lines
925
to
929
| Если ЗначенияМодифицированныхПолей[ИмяПоля_ТребуетсяМониторингПамяти()] = Истина Тогда | ||
| ДобавитьМониторингПамяти(); | ||
| Иначе | ||
| УдалитьМониторингПамяти(); | ||
| КонецЕсли; |
There was a problem hiding this comment.
Не сбрасывайте мониторинг памяти при merge, если поле не модифицировано.
На Line 925-929 ветка Иначе срабатывает и при Неопределено, из-за чего Объединить() отключает мониторинг памяти даже когда исходная конфигурация это поле не меняла.
💡 Предлагаемый фикс
- Если ЗначенияМодифицированныхПолей[ИмяПоля_ТребуетсяМониторингПамяти()] = Истина Тогда
- ДобавитьМониторингПамяти();
- Иначе
- УдалитьМониторингПамяти();
- КонецЕсли;
+ Если Не ЗначенияМодифицированныхПолей[ИмяПоля_ТребуетсяМониторингПамяти()] = Неопределено Тогда
+ Если ЗначенияМодифицированныхПолей[ИмяПоля_ТребуетсяМониторингПамяти()] = Истина Тогда
+ ДобавитьМониторингПамяти();
+ Иначе
+ УдалитьМониторингПамяти();
+ КонецЕсли;
+ КонецЕсли;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Если ЗначенияМодифицированныхПолей[ИмяПоля_ТребуетсяМониторингПамяти()] = Истина Тогда | |
| ДобавитьМониторингПамяти(); | |
| Иначе | |
| УдалитьМониторингПамяти(); | |
| КонецЕсли; | |
| Если ЗначенияМодифицированныхПолей[ИмяПоля_ТребуетсяМониторингПамяти()] <> Неопределено Тогда | |
| Если ЗначенияМодифицированныхПолей[ИмяПоля_ТребуетсяМониторингПамяти()] = Истина Тогда | |
| ДобавитьМониторингПамяти(); | |
| Иначе | |
| УдалитьМониторингПамяти(); | |
| КонецЕсли; | |
| КонецЕсли; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/BenchmarkOneScript/core/Классы/КонфигурацияБенчмарков.os` around lines
925 - 929, При выполнении Объединить() текущая проверка на
ЗначенияМодифицированныхПолей[ИмяПоля_ТребуетсяМониторингПамяти()] = Истина
неверно сбрасывает мониторинг при значении Неопределено; исправьте условие так,
чтобы УдалитьМониторингПамяти() вызывался только когда значение явно = Ложь, а
ДобавитьМониторингПамяти() — когда явно = Истина; то есть заменить конструкцию
If ... Then ... Else ... EndIf на ветвление по явным значениям (If = Истина Then
... ElseIf = Ложь Then ... EndIf) используя массив
ЗначенияМодифицированныхПолей, функцию ИмяПоля_ТребуетсяМониторингПамяти() и
методы ДобавитьМониторингПамяти/УдалитьМониторингПамяти.
Comment on lines
+100
to
+114
| ПрочитанныеНастройки = Новый Соответствие(); | ||
|
|
||
| Для Каждого Каталог Из КаталогиПоиска Цикл | ||
|
|
||
| ПутьКФайлу = ОбъединитьПути(Каталог, ИмяФайла); | ||
| Если Не ФС.ФайлСуществует(ПутьКФайлу) Тогда | ||
| Продолжить; | ||
| КонецЕсли; | ||
|
|
||
| ПрочитатьКонфигурационныйФайл(ПутьКФайлу, ПрочитанныеНастройки); | ||
|
|
||
| КонецЦикла; | ||
|
|
||
| Возврат ПрочитанныеНастройки; | ||
|
|
There was a problem hiding this comment.
Реализация не дополняет конфиг переменными окружения, хотя это заявлено в контракте.
На Lines 100-114 выполняется объединение только файловых источников и сразу Возврат. При этом в комментарии к Прочитать() заявлено дополнение системными переменными окружения. Сейчас это расхождение поведения и контракта.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/BenchmarkOneScript/core/Классы/МенеджерПараметровКонфигурацииOneScript.os`
around lines 100 - 114, Метод собирает настройки только из файлов и сразу
возвращает ПрочитанныеНастройки, но контракт функции Прочитать() обещает
дополнение системными переменными окружения; исправьте это, добавив после цикла
по КаталогиПоиска шаг с чтением и слиянием переменных окружения в
ПрочитанныеНастройки (используйте текущие имена переменных ПрочитанныеНастройки
и функции ПрочитатьКонфигурационныйФайл для поиска места), например пройти по
набору переменных окружения и записать их в ПрочитанныеНастройки, не
перезаписывая уже существующие ключи без явного намерения; затем вернуть
обновлённый ПрочитанныеНастройки.
Comment on lines
+145
to
+151
| Каталоги = СтрРазделить(КаталогиСтрокой, ";"); | ||
| РазрешенныеКаталоги = Новый Массив(); | ||
|
|
||
| Для Каждого Каталог Из Каталоги Цикл | ||
| АбсолютныйПуть = ПолучитьАбсолютныйПуть(Каталог, КаталогРазрешенияПутей); | ||
| РазрешенныеКаталоги.Добавить(АбсолютныйПуть); | ||
| КонецЦикла; |
There was a problem hiding this comment.
Пустые элементы lib.system/lib.additional превращаются в реальные пути.
Если в значении есть ;; или пробельные сегменты, на Lines 145-151 они проходят в ПолучитьАбсолютныйПуть и могут добавить лишний каталог (например, базовый каталог cfg) в путь библиотек.
💡 Предлагаемый фикс
Для Каждого Каталог Из Каталоги Цикл
- АбсолютныйПуть = ПолучитьАбсолютныйПуть(Каталог, КаталогРазрешенияПутей);
+ КаталогБезПробелов = СокрЛП(Каталог);
+ Если Не ЗначениеЗаполнено(КаталогБезПробелов) Тогда
+ Продолжить;
+ КонецЕсли;
+ АбсолютныйПуть = ПолучитьАбсолютныйПуть(КаталогБезПробелов, КаталогРазрешенияПутей);
РазрешенныеКаталоги.Добавить(АбсолютныйПуть);
КонецЦикла;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/BenchmarkOneScript/core/Классы/МенеджерПараметровКонфигурацииOneScript.os`
around lines 145 - 151, При разборе строки каталогов (Каталоги =
СтрРазделить(КаталогиСтрокой, ";")) пустые или состоящие из пробелов сегменты
сейчас передаются в цикл "Для Каждого Каталог Из Каталоги Цикл" и в функцию
ПолучитьАбсолютныйПуть, что приводит к добавлению лишних путей; исправьте это,
пропуская пустые/только-пробельные элементы: в теле цикла предварительно
выполнить обрезку/проверку (trim) переменной Каталог и продолжать цикл
(continue) если результат пустой, прежде чем вызывать ПолучитьАбсолютныйПуть и
добавлять в РазрешенныеКаталоги; оставьте остальную логику с использованием
КаталогРазрешенияПутей без изменений.
Comment on lines
+66
to
+90
| КаталогАртефактов = _МенеджерВременныхФайлов.НовоеИмяФайла(); | ||
| _ЖурналСообщенийПроцесса.Очистить(); | ||
|
|
||
| _КонфигурацияЗапуска.УстановитьКаталогАртефактов(КаталогАртефактов); | ||
|
|
||
| Если ЗначениеЗаполнено(ИсполняющаяСреда.Версия) Тогда | ||
| ФайлИлиВерсия = ИсполняющаяСреда.Версия; | ||
| Версия = ИсполняющаяСреда.Версия; | ||
| Иначе | ||
| ФайлИлиВерсия = _ЛокаторИсполняющейСреды.НайтиИсполняемыйФайлВКаталоге(КаталогПрограммы()); | ||
| Версия = Новый СистемнаяИнформация().Версия; | ||
| КонецЕсли; | ||
|
|
||
| _Лог.Отладка("Запуск процесса в версии <%1> исполняющей среды", Версия); | ||
|
|
||
| ФС.ОбеспечитьПустойКаталог(КаталогАртефактов); | ||
|
|
||
| Процесс = СоздатьИЗапуститьПроцесс(ФайлИлиВерсия, Чекпоинт); | ||
|
|
||
| ОжидатьЗавершениеПроцесса(Процесс); | ||
| ОбработатьВыполнениеПроцесса(Процесс, Версия); | ||
|
|
||
| РезультатЗапуска = ПрочитатьРезультатЗапускаБенчмарковИзJSON(КаталогАртефактов); | ||
|
|
||
| _МенеджерВременныхФайлов.Удалить(); |
There was a problem hiding this comment.
Очистка временных файлов не гарантирована при ошибках после запуска процесса.
Если исключение произойдёт между Line 83 и Line 89, Удалить() на Line 90 не выполнится, и артефакты будут накапливаться.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/BenchmarkOneScript/core/Классы/МенеджерПроцессовБенчмарков.os` around
lines 66 - 90, Поместите удаление временных файлов в блок окончательной
обработки: оберните код от создания/запуска процесса (включая вызовы
СоздатьИЗапуститьПроцесс, ОжидатьЗавершениеПроцесса,
ОбработатьВыполнениеПроцесса и ПрочитатьРезультатЗапускаБенчмарковИзJSON) в
конструкцию try/finally (в 1С — Попытка...Наконец...КонецПопытки) и вызовите
_МенеджерВременныхФайлов.Удалить() внутри раздела Наконец, чтобы гарантировать
удаление артефактов даже при возникновении исключений; не удаляйте логику
обработки ошибок, просто переместите вызов Удалить() в Наконец.
Comment on lines
+280
to
+282
| Если Не ФС.ФайлСуществует(ПутьКОтчету) Тогда | ||
| Возврат Неопределено; | ||
| КонецЕсли; |
There was a problem hiding this comment.
Отсутствующий JSON-отчёт нужно трактовать как ошибку, а не Неопределено.
Сейчас на Line 281 возвращается Неопределено, что скрывает причину сбоя запуска и приводит к нестабильному управляющему циклу выше по стеку.
💡 Предложение исправления
Если Не ФС.ФайлСуществует(ПутьКОтчету) Тогда
- Возврат Неопределено;
+ ВызватьИсключение СтрШаблон(
+ "Не найден файл результата запуска: %1",
+ ПутьКОтчету
+ );
КонецЕсли;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Если Не ФС.ФайлСуществует(ПутьКОтчету) Тогда | |
| Возврат Неопределено; | |
| КонецЕсли; | |
| Если Не ФС.ФайлСуществует(ПутьКОтчету) Тогда | |
| ВызватьИсключение СтрШаблон( | |
| "Не найден файл результата запуска: %1", | |
| ПутьКОтчету | |
| ); | |
| КонецЕсли; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/BenchmarkOneScript/core/Классы/МенеджерПроцессовБенчмарков.os` around
lines 280 - 282, В методе класса МенеджерПроцессовБенчмарков, где проверяется
наличие файла отчёта по переменной ПутьКОтчету, замените возврат Неопределено на
явную ошибку: вместо тихого возврата бросайте исключение или возвращайте
ошибочный результат с описанием (включая путь к файлу) и логированием, чтобы
верхний цикл мог корректно обработать провал; найдите блок с Проверкой
ФС.ФайлСуществует(ПутьКОтчету) и замените поведение на генерацию/возврат ошибки
с понятным сообщением об отсутствии JSON-отчёта.
src/BenchmarkOneScript/core/Классы/СериализаторРезультатовБенчмарков.os
Outdated
Show resolved
Hide resolved
Comment on lines
+9
to
+10
| ИмяФайла = СтрШаблон("%1-report", Строка(РезультатЗапуска.ДескрипторыБенчмарков.ПолучитьПервый().ТипКласса())); | ||
| МаскаФайлов = СтрШаблон("%1.*", ИмяФайла); |
There was a problem hiding this comment.
Добавьте guard на пустую коллекцию дескрипторов перед построением имени файла.
Сейчас на Line 9 вызывается ТипКласса() у ПолучитьПервый() без проверки. При пустой коллекции это приведет к runtime-ошибке и сорвет экспорт.
🛠️ Предложение правки
Процедура Записать(РезультатЗапуска, Знач Каталог = "") Экспорт
-
- ИмяФайла = СтрШаблон("%1-report", Строка(РезультатЗапуска.ДескрипторыБенчмарков.ПолучитьПервый().ТипКласса()));
+
+ ПервыйДескриптор = РезультатЗапуска.ДескрипторыБенчмарков.ПолучитьПервый();
+ Если ПервыйДескриптор = Неопределено Тогда
+ Возврат;
+ КонецЕсли;
+
+ ИмяФайла = СтрШаблон("%1-report", Строка(ПервыйДескриптор.ТипКласса()));
МаскаФайлов = СтрШаблон("%1.*", ИмяФайла);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/BenchmarkOneScript/core/Классы/ЭкспортерРезультатовБенчмарков.os` around
lines 9 - 10, Перед использованием
РезультатЗапуска.ДескрипторыБенчмарков.ПолучитьПервый().ТипКласса() добавьте
защиту на пустую коллекцию: проверьте ДескрипторыБенчмарков на пустоту (напр.,
ДескрипторыБенчмарков.Пуст() или Количество()=0) и в этом случае выберите
безопасное поведение — задать запасное имя файла/маску или корректно прекратить
экспорт с дружелюбной ошибкой; затем только в ветке с ненулевой коллекцией
формируйте ИмяФайла = СтрШаблон("%1-report",
Строка(РезультатЗапуска.ДескрипторыБенчмарков.ПолучитьПервый().ТипКласса())) и
МаскаФайлов = СтрШаблон("%1.*", ИмяФайла).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/BenchmarkOneScript/core/Классы/ДескрипторБенчмарка.os (1)
352-365:⚠️ Potential issue | 🟡 Minor
ДобавитьОдиночныйПараметр()некорректно обрабатывает входПараметрБенчмарка.На Line 364 в новый
ПараметрБенчмаркапередаётся весь объектПараметрБенчмаркакак значение. Для метода с одним параметром это может передать в бенчмарк объект вместоЗначение().💡 Предложение исправления
Процедура ДобавитьОдиночныйПараметр(Параметр) @@ - Параметр = Новый ПараметрБенчмарка(_ИменаПараметров[0], Параметр, Истина); + Если ТипЗнч(Параметр) = Тип("ПараметрБенчмарка") Тогда + Если Не Параметр.ЭтоПараметрМетода() Тогда + ВызватьИсключение "Параметр бенчмарка должен относиться к параметрам метода"; + КонецЕсли; + ЗначениеПараметра = Параметр.Значение(); + Иначе + ЗначениеПараметра = Параметр; + КонецЕсли; + + Параметр = Новый ПараметрБенчмарка(_ИменаПараметров[0], ЗначениеПараметра, Истина);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/Классы/ДескрипторБенчмарка.os` around lines 352 - 365, В процедуре ДобавитьОдиночныйПараметр() при создании нового объекта ПараметрБенчмарка вы передаёте весь объект Параметр как значение, из‑за чего в бенчмарк попадает объект вместо реального значения; исправьте вызов конструктора ПараметрБенчмарка(_ИменаПараметров[0], Параметр, Истина) на передачу фактичесного значения, например Параметр.Значение(), сохранив имя _ИменаПараметров[0] и флаг Истина, и убедитесь, что Параметр поддерживает метод Значение() перед вызовом.
🧹 Nitpick comments (1)
src/BenchmarkOneScript/core/Классы/ДескрипторБенчмарка.os (1)
332-345: Сейчас добавлениеПараметрБенчмаркаделает поверхностное копирование.На Line 332-345 объект параметра добавляется как есть. Это может связать состояния между исходным и копируемым дескриптором (в т.ч. через
Скопировать()), если параметр изменяемый. Лучше нормализовать вход до нового объекта.♻️ Предложение рефакторинга
- Если ТипЗнч(Параметр) = Тип("ПараметрБенчмарка") Тогда - Если Не Параметр.ЭтоПараметрМетода() Тогда - ВызватьИсключение "Параметр бенчмарка должен относиться к параметрам метода"; - КонецЕсли; - Иначе - Параметр = Новый ПараметрБенчмарка( - _ИменаПараметров[ИндексПараметра], - Параметры[ИндексПараметра], - Истина - ); - КонецЕсли; - - ПараметрыМетода.Добавить(Параметр); + Если ТипЗнч(Параметр) = Тип("ПараметрБенчмарка") Тогда + Если Не Параметр.ЭтоПараметрМетода() Тогда + ВызватьИсключение "Параметр бенчмарка должен относиться к параметрам метода"; + КонецЕсли; + ЗначениеПараметра = Параметр.Значение(); + Иначе + ЗначениеПараметра = Параметры[ИндексПараметра]; + КонецЕсли; + + ПараметрыМетода.Добавить(Новый ПараметрБенчмарка( + _ИменаПараметров[ИндексПараметра], + ЗначениеПараметра, + Истина + ));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/Классы/ДескрипторБенчмарка.os` around lines 332 - 345, Сейчас при добавлении в ПараметрыМетода объект ПараметрБенчмарка добавляется по ссылке, что вызывает связанное состояние; вместо этого всегда нормализуйте вход в новый экземпляр ПараметрБенчмарка: в ветке where ТипЗнч(Параметр) = Тип("ПараметрБенчмарка") после проверки ЭтоПараметрМетода() создайте новый ПараметрБенчмарка (копируя нужные поля или вызывая конструктор с именем и значением из существующего) и добавляйте этот новый объект в ПараметрыМетода; в ветке else оставьте создание через Новый ПараметрБенчмарка(...) как сейчас, гарантируя что ПараметрыМетода всегда получает независимый экземпляр.
🤖 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 141-142: Изменить комментарии шагов теста: там где сейчас указан
блок "// Подготовка" перед утверждением
Ожидаем.Что(Результат.Отчет.Таблица).Заполнено(); (в обоих местах — текущие
участки вокруг вызова Ожидаем.Что/Результат.Отчет.Таблица) заменить на "//
Проверка" чтобы правильно отражать, что это проверка/assertion в тесте.
---
Duplicate comments:
In `@src/BenchmarkOneScript/core/Классы/ДескрипторБенчмарка.os`:
- Around line 352-365: В процедуре ДобавитьОдиночныйПараметр() при создании
нового объекта ПараметрБенчмарка вы передаёте весь объект Параметр как значение,
из‑за чего в бенчмарк попадает объект вместо реального значения; исправьте вызов
конструктора ПараметрБенчмарка(_ИменаПараметров[0], Параметр, Истина) на
передачу фактичесного значения, например Параметр.Значение(), сохранив имя
_ИменаПараметров[0] и флаг Истина, и убедитесь, что Параметр поддерживает метод
Значение() перед вызовом.
---
Nitpick comments:
In `@src/BenchmarkOneScript/core/Классы/ДескрипторБенчмарка.os`:
- Around line 332-345: Сейчас при добавлении в ПараметрыМетода объект
ПараметрБенчмарка добавляется по ссылке, что вызывает связанное состояние;
вместо этого всегда нормализуйте вход в новый экземпляр ПараметрБенчмарка: в
ветке where ТипЗнч(Параметр) = Тип("ПараметрБенчмарка") после проверки
ЭтоПараметрМетода() создайте новый ПараметрБенчмарка (копируя нужные поля или
вызывая конструктор с именем и значением из существующего) и добавляйте этот
новый объект в ПараметрыМетода; в ветке else оставьте создание через Новый
ПараметрБенчмарка(...) как сейчас, гарантируя что ПараметрыМетода всегда
получает независимый экземпляр.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5999b003-42cd-469e-8076-81afeef17f3c
📒 Files selected for processing (6)
docs/CLI.mdsrc/BenchmarkOneScript/core/Классы/ДескрипторБенчмарка.ostests/ТестСтратегииХолодныйЗапуск.ostests/ТестыБенчмарков.ostests/ТестыСериализаторНастроекЗапускаБенчмарков.ostests/ТестыСтатистики.os
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/ТестСтратегииХолодныйЗапуск.os
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/BenchmarkOneScript/core/Классы/СериализаторРезультатовБенчмарков.os (1)
40-40:⚠️ Potential issue | 🟡 MinorУберите лишнюю точку с запятой в конце вызова логирования.
На Line 40 есть
;;— выглядит как опечатка и лучше оставить один терминатор инструкции.✏️ Предложение правки
- _Лог.Отладка("Чтение результатов запуска из JSON-файла: %1", ПолноеИмяФайла);; + _Лог.Отладка("Чтение результатов запуска из JSON-файла: %1", ПолноеИмяФайла);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/Классы/СериализаторРезультатовБенчмарков.os` at line 40, В вызове логирования _Лог.Отладка("Чтение результатов запуска из JSON-файла: %1", ПолноеИмяФайла);; удалите лишнюю вторую точку с запятой — оставьте один терминатор инструкции; проверьте метод _Лог.Отладка и строку с ПолноеИмяФайла, чтобы после исправления синтаксис был корректен и не осталось двойных точек с запятой.src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os (1)
121-128:⚠️ Potential issue | 🟡 MinorИспользуется один и тот же чекпоинт при выводе статистики.
На Line 121
Чекпоинтуже перезаписан текущим значением, поэтому на Line 163 сравнение фактически идет “текущий vs текущий”, и переход между кейсами определяется некорректно.💡 Предложение исправления
- Чекпоинт = ТекущийРезультат.Чекпоинт; + ПредыдущийЧекпоинт = Чекпоинт; + Чекпоинт = ТекущийРезультат.Чекпоинт; @@ - ОбработатьИзолированныйЗапуск(РезультатЗапуска, ТекущийРезультат, Чекпоинт); + ОбработатьИзолированныйЗапуск(РезультатЗапуска, ТекущийРезультат, ПредыдущийЧекпоинт, Чекпоинт);-Процедура ОбработатьИзолированныйЗапуск(РезультатЗапуска, ТекущийРезультат, Чекпоинт) +Процедура ОбработатьИзолированныйЗапуск(РезультатЗапуска, ТекущийРезультат, ПредыдущийЧекпоинт, ТекущийЧекпоинт) @@ - СледующийЧекпоинт = ТекущийРезультат.Чекпоинт; @@ - Если ТребуетсяВыводСтатистики(Чекпоинт, СледующийЧекпоинт) Тогда + Если ТребуетсяВыводСтатистики(ПредыдущийЧекпоинт, ТекущийЧекпоинт) Тогда _Репортер.ВывестиСтатистику(ТекущийЗапуск.Статистика); КонецЕсли;Also applies to: 146-164
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os` around lines 121 - 128, Вместо перезаписывать Чекпоинт текущим значением до проверки пересечения кейсов, сохраните предыдущее значение чекпоинта в отдельную переменную (например, ПредыдущийЧекпоинт) до присвоения Чекпоинт = ТекущийРезультат.Чекпоинт или переместите присвоение Чекпоинт после вызова сравнивающей логики; используйте ПредыдущийЧекпоинт при последующем сравнении (там, где сейчас происходит «текущий vs текущий») чтобы правильно детектировать переход между кейсами; измените участки вокруг ПрименитьИнформациюОСредеКЗапускам, ОбработатьИзолированныйЗапуск и вызовы ИзолироватьКаждыйЗапускКейса соответственно.src/BenchmarkOneScript/core/Классы/МенеджерПараметровКонфигурацииOneScript.os (1)
144-150:⚠️ Potential issue | 🟠 MajorНормализуйте и фильтруйте сегменты
lib.system/lib.additionalперед резолвом путей.На Line 148 в
ПолучитьАбсолютныйПутьпередаетсяКаталогбезСокрЛПи проверки заполненности. Сегменты вида" lib2"или" "превращаются в некорректные пути и попадают в итоговыйoscript.cfg.💡 Предлагаемый фикс
Для Каждого Каталог Из Каталоги Цикл - АбсолютныйПуть = ПолучитьАбсолютныйПуть(Каталог, КаталогРазрешенияПутей); + Каталог = СокрЛП(Каталог); + Если Не ЗначениеЗаполнено(Каталог) Тогда + Продолжить; + КонецЕсли; + АбсолютныйПуть = ПолучитьАбсолютныйПуть(Каталог, КаталогРазрешенияПутей); РазрешенныеКаталоги.Добавить(АбсолютныйПуть); КонецЦикла;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/Классы/МенеджерПараметровКонфигурацииOneScript.os` around lines 144 - 150, Нужно нормализовать и отфильтровать каждый сегмент каталога перед вызовом ПолучитьАбсолютныйПуть: для каждого значения из Каталоги (разделённых КаталогиСтрокой) сначала примените СокрЛП/обрезку пробелов и пропустите пустые строки или строки состоящие только из пробелов, а также нормализуйте/сверьте сегменты вроде "lib.system" и "lib.additional" по ожидаемому формату; затем уже вызывать ПолучитьАбсолютныйПуть(НормализованныйКаталог, КаталогРазрешенияПутей) и добавлять результат в РазрешенныеКаталоги, заменив текущую логику в цикле, чтобы не попадали некорректные или пустые пути в итоговый oscript.cfg.
🧹 Nitpick comments (1)
src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os (1)
67-69: Стоит провалидировать значение порта отладки.Сейчас на Line 68 можно передать отрицательное/некорректное значение. Лучше явно ограничить диапазон (например,
0..65535).💡 Предложение исправления
Процедура УстановитьПортОтладки(Порт) Экспорт + Если Порт < 0 Или Порт > 65535 Тогда + ВызватьИсключение "Некорректный порт отладки"; + КонецЕсли; _ПортОтладки = Порт; КонецПроцедуры🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os` around lines 67 - 69, Метод УстановитьПортОтладки сейчас просто присваивает значение в _ПортОтладки без проверок; добавь в начале явную валидацию: убедись, что Порт целое число и находится в диапазоне 0..65535, и при неверном значении либо выбрасывай исключение/ошибку, либо возвращай ошибочный результат (в зависимости от проектных конвенций) — это защитит _ПортОтладки от негативных/некорректных значений.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os`:
- Around line 5-8: The module-level variable _ОбъектБенчмарков is missing from
the initial Перем declaration block which causes a compilation error; add
"_ОбъектБенчмарков; // объект бенчмарков" to the existing Перем list (alongside
_Конфигурация, _ДескрипторыБенчмарков, _Репортер, _ПортОтладки) so the later
assignment to _ОбъектБенчмарков and its use compile correctly.
---
Duplicate comments:
In `@src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os`:
- Around line 121-128: Вместо перезаписывать Чекпоинт текущим значением до
проверки пересечения кейсов, сохраните предыдущее значение чекпоинта в отдельную
переменную (например, ПредыдущийЧекпоинт) до присвоения Чекпоинт =
ТекущийРезультат.Чекпоинт или переместите присвоение Чекпоинт после вызова
сравнивающей логики; используйте ПредыдущийЧекпоинт при последующем сравнении
(там, где сейчас происходит «текущий vs текущий») чтобы правильно детектировать
переход между кейсами; измените участки вокруг
ПрименитьИнформациюОСредеКЗапускам, ОбработатьИзолированныйЗапуск и вызовы
ИзолироватьКаждыйЗапускКейса соответственно.
In
`@src/BenchmarkOneScript/core/Классы/МенеджерПараметровКонфигурацииOneScript.os`:
- Around line 144-150: Нужно нормализовать и отфильтровать каждый сегмент
каталога перед вызовом ПолучитьАбсолютныйПуть: для каждого значения из Каталоги
(разделённых КаталогиСтрокой) сначала примените СокрЛП/обрезку пробелов и
пропустите пустые строки или строки состоящие только из пробелов, а также
нормализуйте/сверьте сегменты вроде "lib.system" и "lib.additional" по
ожидаемому формату; затем уже вызывать
ПолучитьАбсолютныйПуть(НормализованныйКаталог, КаталогРазрешенияПутей) и
добавлять результат в РазрешенныеКаталоги, заменив текущую логику в цикле, чтобы
не попадали некорректные или пустые пути в итоговый oscript.cfg.
In `@src/BenchmarkOneScript/core/Классы/СериализаторРезультатовБенчмарков.os`:
- Line 40: В вызове логирования _Лог.Отладка("Чтение результатов запуска из
JSON-файла: %1", ПолноеИмяФайла);; удалите лишнюю вторую точку с запятой —
оставьте один терминатор инструкции; проверьте метод _Лог.Отладка и строку с
ПолноеИмяФайла, чтобы после исправления синтаксис был корректен и не осталось
двойных точек с запятой.
---
Nitpick comments:
In `@src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os`:
- Around line 67-69: Метод УстановитьПортОтладки сейчас просто присваивает
значение в _ПортОтладки без проверок; добавь в начале явную валидацию: убедись,
что Порт целое число и находится в диапазоне 0..65535, и при неверном значении
либо выбрасывай исключение/ошибку, либо возвращай ошибочный результат (в
зависимости от проектных конвенций) — это защитит _ПортОтладки от
негативных/некорректных значений.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7cb50448-fd46-49c4-93fa-e88234ae8790
📒 Files selected for processing (4)
src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.ossrc/BenchmarkOneScript/core/Классы/МенеджерПараметровКонфигурацииOneScript.ossrc/BenchmarkOneScript/core/Классы/МенеджерПроцессовБенчмарков.ossrc/BenchmarkOneScript/core/Классы/СериализаторРезультатовБенчмарков.os
🚧 Files skipped from review as they are similar to previous changes (1)
- src/BenchmarkOneScript/core/Классы/МенеджерПроцессовБенчмарков.os
Comment on lines
+5
to
+8
| Перем _Конфигурация; // КонфигурацияБенчмарков | ||
| Перем _ДескрипторыБенчмарков; // КоллекцияДескрипторовБенчмарков | ||
| Перем _Репортер; // КонсольныйРепортерБенчмарков | ||
| Перем _ЛокаторИсполняющейСреды; // ЛокаторИсполняющейСреды | ||
| Перем _МенеджерВременныхФайлов; // МенеджерВременныхФайлов | ||
| Перем _Лог; // Лог | ||
| Перем _ЖурналСообщенийПроцесса; // Массив из Строка | ||
| Перем _Репортер; // КонсольныйРепортерБенчмарков, НулевойРепортерБенчмарков | ||
| Перем _ПортОтладки; // Число |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd 'ИзолированныйЗапускательБенчмарков\.os$'Repository: Stivo182/BenchmarkOneScript
Length of output: 143
🏁 Script executed:
head -n 50 "src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os" | cat -nRepository: Stivo182/BenchmarkOneScript
Length of output: 1959
Add _ОбъектБенчмарков declaration at module level.
Variable is assigned on line 24 and used on line 32, but is not declared in the Перем block (lines 5-8). This will cause a compilation error in OneScript.
💡 Suggested fix
Перем _Конфигурация; // КонфигурацияБенчмарков
Перем _ДескрипторыБенчмарков; // КоллекцияДескрипторовБенчмарков
+Перем _ОбъектБенчмарков; // Произвольный - экземпляр класса с бенчмарками
Перем _Репортер; // КонсольныйРепортерБенчмарков, НулевойРепортерБенчмарков
Перем _ПортОтладки; // Число📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Перем _Конфигурация; // КонфигурацияБенчмарков | |
| Перем _ДескрипторыБенчмарков; // КоллекцияДескрипторовБенчмарков | |
| Перем _Репортер; // КонсольныйРепортерБенчмарков | |
| Перем _ЛокаторИсполняющейСреды; // ЛокаторИсполняющейСреды | |
| Перем _МенеджерВременныхФайлов; // МенеджерВременныхФайлов | |
| Перем _Лог; // Лог | |
| Перем _ЖурналСообщенийПроцесса; // Массив из Строка | |
| Перем _Репортер; // КонсольныйРепортерБенчмарков, НулевойРепортерБенчмарков | |
| Перем _ПортОтладки; // Число | |
| Перем _Конфигурация; // КонфигурацияБенчмарков | |
| Перем _ДескрипторыБенчмарков; // КоллекцияДескрипторовБенчмарков | |
| Перем _ОбъектБенчмарков; // Произвольный - экземпляр класса с бенчмарками | |
| Перем _Репортер; // КонсольныйРепортерБенчмарков, НулевойРепортерБенчмарков | |
| Перем _ПортОтладки; // Число |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os`
around lines 5 - 8, The module-level variable _ОбъектБенчмарков is missing from
the initial Перем declaration block which causes a compilation error; add
"_ОбъектБенчмарков; // объект бенчмарков" to the existing Перем list (alongside
_Конфигурация, _ДескрипторыБенчмарков, _Репортер, _ПортОтладки) so the later
assignment to _ОбъектБенчмарков and its use compile correctly.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/BenchmarkOneScript/core/Классы/ВалидаторЗапускаБенчмарков.os (1)
25-33:⚠️ Potential issue | 🟠 MajorДобавьте раннюю проверку на пустой список бенчмарков.
Сейчас при пустом наборе дескрипторов валидация может пройти, а падение произойдет позже (например, при обращении к первому дескриптору в генерации сценария). Лучше вернуть понятную ошибку сразу на этапе
ЗапускВозможен().💡 Предложение исправления
Функция ЗапускВозможен() Экспорт _СписокОшибок.Очистить(); + ПроверитьНаличиеБенчмарков(); ПроверитьТипыБенчмарков(); ПроверитьЭталоннуюВерсиюИсполняющейСреды(); ПроверитьЭталонныеБенчмарки(); ПроверитьПараметрыНаВозможностьСериализации(); @@ КонецФункции + +Процедура ПроверитьНаличиеБенчмарков() + Если _ДескрипторыБенчмарков.ВМассив().Количество() = 0 Тогда + _СписокОшибок.Добавить("Не найдено ни одного бенчмарка для запуска"); + КонецЕсли; +КонецПроцедуры🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/Классы/ВалидаторЗапускаБенчмарков.os` around lines 25 - 33, В функции Экспортная ЗапускВозможен() добавьте раннюю проверку на пустой список дескрипторов бенчмарков (например переменная, содержащая набор дескрипторов, используемая в классе) перед вызовами ПроверитьТипыБенчмарков(), ПроверитьЭталоннуюВерсиюИсполняющейСреды(), ПроверитьЭталонныеБенчмарки() и ПроверитьПараметрыНаВозможностьСериализации(): если список пуст — добавить понятную запись об ошибке в _СписокОшибок (например "Нет дескрипторов бенчмарков для запуска") и прекратить дальнейшую валидацию (вернуть false или соответствующий код ошибки), чтобы избежать обращения к первому дескриптору в последующих методах.
♻️ Duplicate comments (1)
src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os (1)
121-128:⚠️ Potential issue | 🟠 MajorСохраните предыдущий checkpoint до перезаписи, иначе сравниваются одинаковые значения.
На Line 121 текущее значение checkpoint уже затирается, и на Line 163 в
ТребуетсяВыводСтатистики(...)попадают одинаковые аргументы.💡 Предложение исправления
- Чекпоинт = ТекущийРезультат.Чекпоинт; + ПредыдущийЧекпоинт = Чекпоинт; + Чекпоинт = ТекущийРезультат.Чекпоинт; @@ - ОбработатьИзолированныйЗапуск(РезультатЗапуска, ТекущийРезультат, Чекпоинт); + ОбработатьИзолированныйЗапуск(РезультатЗапуска, ТекущийРезультат, ПредыдущийЧекпоинт, Чекпоинт); @@ -Процедура ОбработатьИзолированныйЗапуск(РезультатЗапуска, ТекущийРезультат, Чекпоинт) +Процедура ОбработатьИзолированныйЗапуск(РезультатЗапуска, ТекущийРезультат, ПредыдущийЧекпоинт, ТекущийЧекпоинт) @@ - СледующийЧекпоинт = ТекущийРезультат.Чекпоинт; @@ - Если ТребуетсяВыводСтатистики(Чекпоинт, СледующийЧекпоинт) Тогда + Если ТребуетсяВыводСтатистики(ПредыдущийЧекпоинт, ТекущийЧекпоинт) Тогда _Репортер.ВывестиСтатистику(ТекущийЗапуск.Статистика); КонецЕсли;Also applies to: 146-164
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os` around lines 121 - 128, Сохраните значение чекпоинта до вызовов, которые могут изменить ТекущийРезультат: сразу после чтения чекпоинта присвойте временную переменную (например, ПредыдущийЧекпоинт = ТекущийРезультат.Чекпоинт) и используйте эту переменную при вызове ОбработатьИзолированныйЗапуск и/или ТребуетсяВыводСтатистики вместо повторного обращения к ТекущийРезультат.Чекпоинт; это гарантирует, что сравниваются разные значения и предотвращает передачу одинаковых аргументов после ПрименитьИнформациюОСредеКЗапускам/изменений в ТекущийРезультат.
🤖 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`:
- Line 321: Переименовать процедуру
ТестДолжен_ВыброситьИсключениеПриПередачиОдиночногоПараметраКогдаМетодИмеетНесколькоПараметров
в правильную форму
ТестДолжен_ВыброситьИсключениеПриПередачеОдиночногоПараметраКогдаМетодИмеетНесколькоПараметров:
обновить объявление процедуры и все её вызовы/ссылки в проекте (поиск по имени
процедуры) чтобы избежать broken references и обеспечить корректное соглашение
об именовании.
- Around line 343-344: Переименуйте процедуру
ТестДолжен_ВыброситьИсключениеПриПараметраБенчмаркаОбщегоПоля в грамматически
корректное имя (например
ТестДолжен_ВыброситьИсключениеПриДобавленииПараметраОбщегоПоля или
ТестДолжен_ВыброситьИсключениеПриИспользованииПараметраОбщегоПоляКакПараметраМетода)
и обновите все её объявления/экспорты и вызовы/ссылки в тестах и регистрациях,
сохранив экспорт (Экспорт) и поведение процедуры; убедитесь, что имя в сигнатуре
процедуры и во всех местах вызова совпадают.
---
Outside diff comments:
In `@src/BenchmarkOneScript/core/Классы/ВалидаторЗапускаБенчмарков.os`:
- Around line 25-33: В функции Экспортная ЗапускВозможен() добавьте раннюю
проверку на пустой список дескрипторов бенчмарков (например переменная,
содержащая набор дескрипторов, используемая в классе) перед вызовами
ПроверитьТипыБенчмарков(), ПроверитьЭталоннуюВерсиюИсполняющейСреды(),
ПроверитьЭталонныеБенчмарки() и ПроверитьПараметрыНаВозможностьСериализации():
если список пуст — добавить понятную запись об ошибке в _СписокОшибок (например
"Нет дескрипторов бенчмарков для запуска") и прекратить дальнейшую валидацию
(вернуть false или соответствующий код ошибки), чтобы избежать обращения к
первому дескриптору в последующих методах.
---
Duplicate comments:
In `@src/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.os`:
- Around line 121-128: Сохраните значение чекпоинта до вызовов, которые могут
изменить ТекущийРезультат: сразу после чтения чекпоинта присвойте временную
переменную (например, ПредыдущийЧекпоинт = ТекущийРезультат.Чекпоинт) и
используйте эту переменную при вызове ОбработатьИзолированныйЗапуск и/или
ТребуетсяВыводСтатистики вместо повторного обращения к
ТекущийРезультат.Чекпоинт; это гарантирует, что сравниваются разные значения и
предотвращает передачу одинаковых аргументов после
ПрименитьИнформациюОСредеКЗапускам/изменений в ТекущийРезультат.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3c761574-491f-4598-8194-0c3ac5afb29b
📒 Files selected for processing (10)
src/BenchmarkOneScript/core/Классы/ВалидаторЗапускаБенчмарков.ossrc/BenchmarkOneScript/core/Классы/ДескрипторБенчмарка.ossrc/BenchmarkOneScript/core/Классы/ЗапускательБенчмарков.ossrc/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.ossrc/BenchmarkOneScript/core/Классы/ОберткаИсточникаБенчмарков.ossrc/BenchmarkOneScript/core/Классы/РезолверБенчмарков.ossrc/BenchmarkOneScript/core/Классы/СериализаторНастроекЗапускаБенчмарков.ossrc/BenchmarkOneScript/core/Классы/СериализаторРезультатовБенчмарков.ossrc/BenchmarkOneScript/core/Классы/СценарийЗапускаБенчмарка.ostests/ТестыБенчмарков.os
💤 Files with no reviewable changes (1)
- src/BenchmarkOneScript/core/Классы/ОберткаИсточникаБенчмарков.os
🚧 Files skipped from review as they are similar to previous changes (1)
- src/BenchmarkOneScript/core/Классы/СериализаторНастроекЗапускаБенчмарков.os
tests/ТестыБенчмарков.os
Outdated
|
|
||
| &Тест | ||
| Процедура Тест_ПроверяетОшибкуПередачиОдиночногоПараметраКогдаМетодИмеетНесколькоПараметров() Экспорт | ||
| Процедура ТестДолжен_ВыброситьИсключениеПриПередачиОдиночногоПараметраКогдаМетодИмеетНесколькоПараметров() Экспорт |
There was a problem hiding this comment.
Опечатка в названии процедуры.
В названии ПриПередачиОдиночногоПараметра используется родительный падеж вместо предложного. Правильно: ПриПередачеОдиночногоПараметра (при чём? — при передаче).
📝 Предлагаемое исправление
-Процедура ТестДолжен_ВыброситьИсключениеПриПередачиОдиночногоПараметраКогдаМетодИмеетНесколькоПараметров() Экспорт
+Процедура ТестДолжен_ВыброситьИсключениеПриПередачеОдиночногоПараметраКогдаМетодИмеетНесколькоПараметров() Экспорт📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Процедура ТестДолжен_ВыброситьИсключениеПриПередачиОдиночногоПараметраКогдаМетодИмеетНесколькоПараметров() Экспорт | |
| Процедура ТестДолжен_ВыброситьИсключениеПриПередачеОдиночногоПараметраКогдаМетодИмеетНесколькоПараметров() Экспорт |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/ТестыБенчмарков.os` at line 321, Переименовать процедуру
ТестДолжен_ВыброситьИсключениеПриПередачиОдиночногоПараметраКогдаМетодИмеетНесколькоПараметров
в правильную форму
ТестДолжен_ВыброситьИсключениеПриПередачеОдиночногоПараметраКогдаМетодИмеетНесколькоПараметров:
обновить объявление процедуры и все её вызовы/ссылки в проекте (поиск по имени
процедуры) чтобы избежать broken references и обеспечить корректное соглашение
об именовании.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/ТестыБенчмарков.os (1)
143-147:⚠️ Potential issue | 🟠 MajorСтабилизируйте проверки без привязки к индексу строки отчета.
На Line 143-147, Line 165-176, Line 205-208 и Line 232-235 проверки завязаны на
Таблица[0..n]. Для запуска итераций в отдельных процессах это риск flaky-тестов из-за потенциально недетерминированного порядка результатов. Лучше явно сортировать таблицу перед assert или искать строки по значениям параметров.Пример стабилизации через явную сортировку
Результат = Бенчмаркинг.Запустить(Тип("БенчмаркСПараметрамиМетода"), Конфигурация); НаборыПараметров = Результат.ДескрипторыБенчмарков.ПолучитьПервый().НаборыПараметров(); + ИмяПарам1 = КолонкиОтчетаБенчмарков.Параметр("Парам1"); + ИмяПарам2 = КолонкиОтчетаБенчмарков.Параметр("Парам2"); + Результат.Отчет.Таблица.Сортировать(ИмяПарам1 + "," + ИмяПарам2); // Проверка Ожидаем.Что(Результат.Отчет.Таблица).Заполнено(); - Ожидаем.Что(Результат.Отчет.Таблица[0][КолонкиОтчетаБенчмарков.Параметр("Парам1")]).Равно(10); - Ожидаем.Что(Результат.Отчет.Таблица[0][КолонкиОтчетаБенчмарков.Параметр("Парам2")]).Равно(100); + Ожидаем.Что(Результат.Отчет.Таблица[0][ИмяПарам1]).Равно(10); + Ожидаем.Что(Результат.Отчет.Таблица[0][ИмяПарам2]).Равно(100);Also applies to: 165-176, 205-208, 232-235
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыБенчмарков.os` around lines 143 - 147, Проверки завязаны на индекс строки в Результат.Отчет.Таблица и поэтому флакуют; измените их в тестах (например рядом с вызовом проверок для Таблица) так, чтобы сначала стабилизировать порядок — либо явно отсортировать Результат.Отчет.Таблица по ключу (например по колонке, получаемой через КолонкиОтчетаБенчмарков.Параметр) перед ассертами, либо вместо обращения по индексу искать нужную строку по значению параметра (сравнивать значение колонки через КолонкиОтчетаБенчмарков.Параметр("Парам1")/("Парам2") и затем проверять её значения); также примените ту же правку к другим блокам, где используются Таблица[0..n] и к проверке НаборыПараметров[0].Получить(0).ЭтоПараметрМетода(), чтобы адресация не зависела от порядка элементов.
🧹 Nitpick comments (2)
src/BenchmarkOneScript/dto/Классы/РезультатЗапускаБенчмаркаДто.os (1)
60-68: Сделайте явный возврат при отсутствии замера.Сейчас при ненайденной итерации функция полагается на неявный
Неопределено. Лучше вернуть его явно для предсказуемого публичного API.Предложение
Функция НайтиИтерациюЗамера(Этап, НомерИтерации) Экспорт Для Каждого Замер Из Замеры Цикл Если Замер.Этап = Этап И Замер.НомерИтерации = НомерИтерации Тогда Возврат Замер; КонецЕсли; КонецЦикла; + + Возврат Неопределено; КонецФункции🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/dto/Классы/РезультатЗапускаБенчмаркаДто.os` around lines 60 - 68, The function НайтиИтерациюЗамера currently falls through without an explicit return when no matching measurement is found; update НайтиИтерациюЗамера(Этап, НомерИтерации) to explicitly return Неопределено at the end of the function after the loop (after iterating over Замеры) so callers get a predictable value when no Замер with matching Этап and НомерИтерации is found.tests/ТестыБенчмарков.os (1)
226-239: Добавьте проверку, чтоСкопировать()делает именно независимую копию.Сейчас проверяется равенство данных, но не отсутствие разделяемого состояния между оригиналом и копией после изменений.
Минимальное усиление теста deep-copy
// Действие КопияДескриптора = Дескриптор.Скопировать(); + КопияДескриптора.ДобавитьИсточникПараметров("Источник3"); // Проверка НаборыПараметров = КопияДескриптора.НаборыПараметров(); ИсточникиПараметров = КопияДескриптора.ИсточникиПараметров(); + Ожидаем.Что(Дескриптор.ИсточникиПараметров().Количество()).Равно(2); + Ожидаем.Что(ИсточникиПараметров.Количество()).Равно(3);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыБенчмарков.os` around lines 226 - 239, Current assertions only check value equality after Скопировать(); add checks to ensure the copy is independent by mutating the original and verifying the copy does not change: call methods on Дескриптор to modify a parameter value, источник или флаг (e.g., change Дескриптор.НаборыПараметров()[0].Получить(0).Установить(...) or toggle Дескриптор.ЭтоЭталон()), then assert that КопияДескриптора.НаборыПараметров()/ИсточникиПараметров()/ЭтоЭталон() still hold the original values (use the existing Ожидаем.Что(...).Равно/ЭтоИстина helpers); likewise mutate КопияДескриптора and assert Дескриптор remains unchanged to prove deep-copy behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@tests/ТестыБенчмарков.os`:
- Around line 143-147: Проверки завязаны на индекс строки в
Результат.Отчет.Таблица и поэтому флакуют; измените их в тестах (например рядом
с вызовом проверок для Таблица) так, чтобы сначала стабилизировать порядок —
либо явно отсортировать Результат.Отчет.Таблица по ключу (например по колонке,
получаемой через КолонкиОтчетаБенчмарков.Параметр) перед ассертами, либо вместо
обращения по индексу искать нужную строку по значению параметра (сравнивать
значение колонки через КолонкиОтчетаБенчмарков.Параметр("Парам1")/("Парам2") и
затем проверять её значения); также примените ту же правку к другим блокам, где
используются Таблица[0..n] и к проверке
НаборыПараметров[0].Получить(0).ЭтоПараметрМетода(), чтобы адресация не зависела
от порядка элементов.
---
Nitpick comments:
In `@src/BenchmarkOneScript/dto/Классы/РезультатЗапускаБенчмаркаДто.os`:
- Around line 60-68: The function НайтиИтерациюЗамера currently falls through
without an explicit return when no matching measurement is found; update
НайтиИтерациюЗамера(Этап, НомерИтерации) to explicitly return Неопределено at
the end of the function after the loop (after iterating over Замеры) so callers
get a predictable value when no Замер with matching Этап and НомерИтерации is
found.
In `@tests/ТестыБенчмарков.os`:
- Around line 226-239: Current assertions only check value equality after
Скопировать(); add checks to ensure the copy is independent by mutating the
original and verifying the copy does not change: call methods on Дескриптор to
modify a parameter value, источник или флаг (e.g., change
Дескриптор.НаборыПараметров()[0].Получить(0).Установить(...) or toggle
Дескриптор.ЭтоЭталон()), then assert that
КопияДескриптора.НаборыПараметров()/ИсточникиПараметров()/ЭтоЭталон() still hold
the original values (use the existing Ожидаем.Что(...).Равно/ЭтоИстина helpers);
likewise mutate КопияДескриптора and assert Дескриптор remains unchanged to
prove deep-copy behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 783221ab-e54c-45d8-a436-ca70638505dd
📒 Files selected for processing (3)
src/BenchmarkOneScript/core/Классы/РезультатЗапускаБенчмарков.ossrc/BenchmarkOneScript/dto/Классы/РезультатЗапускаБенчмаркаДто.ostests/ТестыБенчмарков.os
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
Improvements
Tests