fix: Исправлено сопоставление параметров бенчмарков с эталонными кейсами#41
Open
fix: Исправлено сопоставление параметров бенчмарков с эталонными кейсами#41
Conversation
📝 WalkthroughWalkthroughRefactors the runner to a case-centric model: adds КейсБенчмарка and generators, replaces РезультатЗапускаБенчмарка with РезультатЗапускаКейса, converts environment storage/API (Версии→ИсполняющиеСреды), replaces lazy delegate with ФабрикаДелегата, rewrites launchers, parameter reader, serializers and updates tests/fixtures. Changes
Sequence Diagram(s)sequenceDiagram
participant Generator as ГенераторКейсов
participant Launcher as ЗапускательБенчмарков
participant ProcMgr as МенеджерПроцессов
participant Runner as ВнешнийПроцесс/Исполнитель
participant Report as ПостроительОтчета
Generator->>Launcher: СгенерироватьВсеКейсы(Конфигурация, Дескрипторы) -> Кейсы
Launcher->>ProcMgr: ЗапуститьПроцесс(Кейсы, ИсполняющаяСреда, Чекпоинт)
ProcMgr->>Runner: СоздатьИЗапуститьПроцесс(Версия, Кейсы, Чекпоинт)
Runner->>Runner: ВыполнитьКейсы(Кейс...)
Runner-->>ProcMgr: ВернутьРезультаты(РезультатЗапускаКейса...)
ProcMgr-->>Launcher: ПередатьРезультаты(РезультатЗапускаКейса...)
Launcher->>Report: СформироватьОтчет(Результаты)
Report-->>Launcher: Отчет
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/fixtures/verified-report.json (1)
86-150: 🛠️ Refactor suggestion | 🟠 MajorThis golden report still doesn’t exercise the baseline-case mapping fix.
Both runs here are for different methods and both keep
Baseline: false, so the fixture never creates the ambiguous case the PR is supposed to fix: multiple cases of the same benchmark where one reference/baseline case must be matched with the right parameter set. Please add at least one same-Methodpair with distinct parameters and a baseline case, otherwise this regression can come back unnoticed.Also applies to: 246-304
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/fixtures/verified-report.json` around lines 86 - 150, The fixture lacks a same-Method pair with differing Parameters and a Baseline=true case, so add another case object with the same "Method" value as an existing case (e.g., "БенчмаркПервый"), give it a different "CaseId" and different "Parameters" values, and set its "Baseline" field to true so the test exercises the baseline-case mapping; ensure the original case keeps Baseline:false and that both cases include matching "Method" and comparable "Parameters" shapes to trigger the ambiguous-match logic (apply same change to the other block indicated in the file).
🧹 Nitpick comments (2)
src/BenchmarkOneScript/core/Классы/РезультатИтерацииКейса.os (1)
45-94: Consider using a computed padding instead of hardcoded spaces.The padding string on line 59 (
" "- 14 spaces) is hardcoded. While currently functional, this could become fragile if stage name lengths change.♻️ Optional: Use computed padding based on ДлинаЭтапа
- Значение = СтрШаблон("%1 %2: %3 op, ", - Лев(Этап + " ", ДлинаЭтапа), + ПробельнаяСтрока = ""; + Для Сч = 1 По ДлинаЭтапа Цикл + ПробельнаяСтрока = ПробельнаяСтрока + " "; + КонецЦикла; + + Значение = СтрШаблон("%1 %2: %3 op, ", + Лев(Этап + ПробельнаяСтрока, ДлинаЭтапа),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/Классы/РезультатИтерацииКейса.os` around lines 45 - 94, The hardcoded padding (" ") in ОбработкаПолученияПредставления is fragile; replace it with a computed padding using ДлинаЭтапа so the stage name padding adapts to changes. Locate the Лев call that currently does Лев(Этап + " ", ДлинаЭтапа) and change it to concatenate a generated space string (e.g., via СтрПовтор(" ", ДлинаЭтапа) or similar) or otherwise ensure you pass a padded string of length ДлинаЭтапа to Лев so the alignment uses the computed ДлинаЭтапа instead of fixed spaces.src/BenchmarkOneScript/core/Классы/ФабрикаДелегата.os (1)
77-85: Consider using explicitНеопределеноcheck instead ofЗначениеЗаполнено.
ЗначениеЗаполнено(_Объект)returnsЛожьfor values like0, empty string"", or empty date. If a valid object with such a value is passed, the validation will incorrectly fail. Consider:♻️ Suggested fix
Функция СоздатьДелегатДляМетодаОбъекта() - Если Не ЗначениеЗаполнено(_Объект) Тогда + Если _Объект = Неопределено Тогда ВызватьИсключение СтрШаблон("Для создания делегата <%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 77 - 85, В функции СоздатьДелегатДляМетодаОбъекта замените текущую проверку ЗначениеЗаполнено(_Объект) на явную проверку на Неопределено (например: Если _Объект = Неопределено Тогда ...), чтобы не отвергать допустимые «ложные» значения (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 `@docs/ОбработчикиСобытий.md`:
- Line 46: В документации для аннотации ПередКаждым замените неверное имя типа
ДКейсБенчмарка на корректное КейсБенчмарка: откройте секцию, где описан
ПередКаждым (строка с "Кейс - ДКейсБенчмарка") и исправьте символ типа на
КейсБенчмарка, чтобы соответствовать реальному классу КейсБенчмарка,
используемому в кодовой базе и в АннотацияПередКаждым.os.
In `@src/BenchmarkOneScript/cmd/Классы/КомандаЗапуска.os`:
- Around line 255-257: Строки, полученные через СтрРазделить(_ИсполняющаяСреда,
","), могут содержать ведущие/замыкающие пробелы, поэтому перед вызовом
Конфигурация.ДобавитьИсполняющуюСреду(Версия) нужно нормализовать каждую Версия
— удалить пробелы в начале и конце (trim) и пропускать пустые элементы; внесите
это прямо в цикл, где формируется ИсполняющиеСреды и обрабатывается переменная
Версия.
In `@src/BenchmarkOneScript/core/Классы/ЗапускательБенчмарков.os`:
- Around line 66-69: Вызов метода Запустить(Кейсы = Неопределено) сейчас не
проверяет переданные кейсы — добавьте проверку входного параметра перед
созданием/запуском сценариев: либо расширьте класс ВалидаторЗапускаБенчмарков
(например, добавить метод ПроверитьКейсы/ValidateCasesAgainstDescriptors) чтобы
сравнивать каждую запись в Кейсы с текущими _ДескрипторыБенчмарков и
_Конфигурация и подтвердить наличие всех обязательных параметров, либо вызовите
существующий валидатор для Кейсы до начала вывода/хуков; при ошибке валидации
немедленно прекращайте выполнение (кинуть исключение или вернуть ошибку) до
вызова ЗапуститьКейс() и любых побочных эффектов.
- Around line 142-159: ЗапуститьКейсыБенчмарков регенерирует кейсы и сразу
итеративно проходит их, но пропуск по чекпоинту применяется только внутри
итерации, из‑за чего при холодном старте цикл может застревать на первом кейсе;
перед вызовом ПроверитьИСоздатьЧекпоинт(Кейс) добавь проверку уровня кейса —
если _Чекпоинт не определён или _БылоВыполнениеИтерации = Истина — не
пропускать, иначе сравни _Чекпоинт.ИдентификаторКейса с Кейс.Идентификатор() и
при несовпадении сразу продолжай цикл (пропусти этот Кейс); можно вынести логику
в функцию ТребуетсяПропуститьКейс(Кейс) и использовать её до вызова
ПроверитьИСоздатьЧекпоинт.
In `@src/BenchmarkOneScript/core/Классы/ИсполняющаяСредаБенчмарков.os`:
- Around line 11-19: The change renames the serialized runtime shape to
"Baseline" and removes the previously exported alias field, which breaks
downstream JSON consumers expecting Runtime.Alias; restore backward
compatibility by keeping the old serialized name or emitting both fields: keep
the new &Сериализуемое("Baseline") annotation for the new schema but also
serialize the legacy key "Alias" (map the value from the variable ЭтоЭталон) so
both Runtime.Baseline and Runtime.Alias exist during a deprecation window, and
update Процедура ПриСозданииОбъекта() initialization/documentation or add
explicit schema versioning metadata to signal the change.
In `@src/BenchmarkOneScript/core/Классы/КонфигурацияБенчмарков.os`:
- Around line 795-804: В функции ДобавитьИсполняющуюСреду(Версия, Наименование =
"", ЭтоЭталон = Ложь) предотвратите добавление более чем одной эталонной среды:
перед _ИсполняющиеСреды.Добавить(...) при ЭтоЭталон = Истина пройдитесь по
коллекции _ИсполняющиеСреды и если найдена уже существующая запись с полем
ЭтоЭталон = Истина — прервите операцию и вызовите Ошибка("Уже существует
эталонная исполняющая среда") или верните/логируйте соответствующее сообщение;
это гарантирует, что метод ЭталоннаяИсполняющаяСреда() не будет полагаться на
порядок добавления и сохранит инвариант единственной эталонной среды.
In `@src/BenchmarkOneScript/core/Классы/РезультатЗапускаБенчмарков.os`:
- Around line 26-27: The region name header contains a typo: change the region
token "ОбработчикиСобыий" to the correct spelling "ОбработчикиСобытий" (update
the "#Область ОбработчикиСобыий" declaration and any matching occurrences in
this file such as region close tags or references to ensure they match exactly),
keeping the region markers (e.g., "#Область" / "#КонецОбласти") consistent.
In `@src/BenchmarkOneScript/core/Классы/ЧитательПараметровБенчмарков.os`:
- Line 31: There's a typo in the local variable name: replace the incorrect
ЗначеЗначенияИзИсточника with ЗначенияИзИсточника where it's assigned from
ПолучитьЗначенияИзИсточника(Источник) and update any subsequent uses (e.g., the
reference on the following line) to the corrected identifier so all occurrences
match the new name.
---
Outside diff comments:
In `@tests/fixtures/verified-report.json`:
- Around line 86-150: The fixture lacks a same-Method pair with differing
Parameters and a Baseline=true case, so add another case object with the same
"Method" value as an existing case (e.g., "БенчмаркПервый"), give it a different
"CaseId" and different "Parameters" values, and set its "Baseline" field to true
so the test exercises the baseline-case mapping; ensure the original case keeps
Baseline:false and that both cases include matching "Method" and comparable
"Parameters" shapes to trigger the ambiguous-match logic (apply same change to
the other block indicated in the file).
---
Nitpick comments:
In `@src/BenchmarkOneScript/core/Классы/РезультатИтерацииКейса.os`:
- Around line 45-94: The hardcoded padding (" ") in
ОбработкаПолученияПредставления is fragile; replace it with a computed padding
using ДлинаЭтапа so the stage name padding adapts to changes. Locate the Лев
call that currently does Лев(Этап + " ", ДлинаЭтапа) and change it
to concatenate a generated space string (e.g., via СтрПовтор(" ", ДлинаЭтапа) or
similar) or otherwise ensure you pass a padded string of length ДлинаЭтапа to
Лев so the alignment uses the computed ДлинаЭтапа instead of fixed spaces.
In `@src/BenchmarkOneScript/core/Классы/ФабрикаДелегата.os`:
- Around line 77-85: В функции СоздатьДелегатДляМетодаОбъекта замените текущую
проверку ЗначениеЗаполнено(_Объект) на явную проверку на Неопределено (например:
Если _Объект = Неопределено Тогда ...), чтобы не отвергать допустимые «ложные»
значения (0, "", пустая дата) как отсутствующий объект; оставьте создание
делегата через Новый Действие(_Объект, _КвалифицированноеИмяМетода) без
изменений.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a06215f2-e128-4633-985b-e66b387908c4
📒 Files selected for processing (71)
docs/ОбработчикиСобытий.mddocs/Параметризация.mddocs/СредаВыполнения.mddocs/Эталоны.mdsamples/api/ЗапускБенчмарковВРазныхВерсияхOneScript.ossrc/BenchmarkOneScript/annotations/Классы/АннотацияИсточникПараметров.ossrc/BenchmarkOneScript/annotations/Классы/АннотацияПередКаждым.ossrc/BenchmarkOneScript/annotations/Классы/АннотацияПослеВсех.ossrc/BenchmarkOneScript/annotations/Классы/АннотацияПослеКаждого.ossrc/BenchmarkOneScript/cmd/Классы/КомандаЗапуска.ossrc/BenchmarkOneScript/columns/Модули/КолонкаОтчетаБенчмарковRatioSD.ossrc/BenchmarkOneScript/columns/Модули/КолонкаОтчетаБенчмарковВерсияИсполняющейСреды.ossrc/BenchmarkOneScript/columns/Модули/КолонкаОтчетаБенчмарковВерхнийКвартиль.ossrc/BenchmarkOneScript/columns/Модули/КолонкаОтчетаБенчмарковВыделяемаяПамять.ossrc/BenchmarkOneScript/columns/Модули/КолонкаОтчетаБенчмарковКатегория.ossrc/BenchmarkOneScript/columns/Модули/КолонкаОтчетаБенчмарковКоэффициентПроизводительности.ossrc/BenchmarkOneScript/columns/Модули/КолонкаОтчетаБенчмарковМакс.ossrc/BenchmarkOneScript/columns/Модули/КолонкаОтчетаБенчмарковМедиана.ossrc/BenchmarkOneScript/columns/Модули/КолонкаОтчетаБенчмарковМетод.ossrc/BenchmarkOneScript/columns/Модули/КолонкаОтчетаБенчмарковМин.ossrc/BenchmarkOneScript/columns/Модули/КолонкаОтчетаБенчмарковНижнийКвартиль.ossrc/BenchmarkOneScript/columns/Модули/КолонкаОтчетаБенчмарковОперацийВСекунду.ossrc/BenchmarkOneScript/columns/Модули/КолонкаОтчетаБенчмарковПараметр.ossrc/BenchmarkOneScript/columns/Модули/КолонкаОтчетаБенчмарковПроцентиль.ossrc/BenchmarkOneScript/columns/Модули/КолонкаОтчетаБенчмарковСреднее.ossrc/BenchmarkOneScript/columns/Модули/КолонкаОтчетаБенчмарковСтандартнаяОшибка.ossrc/BenchmarkOneScript/columns/Модули/КолонкаОтчетаБенчмарковСтандартноеОтклонение.ossrc/BenchmarkOneScript/columns/Модули/КолонкаОтчетаБенчмарковЭталон.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/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/core/Модули/ИзвлекательКонфигурацииБенчмарков.ossrc/BenchmarkOneScript/core/Модули/СобытияБенчмарков.ossrc/BenchmarkOneScript/dto/Классы/РезультатИтерацииБенчмаркаДто.ostests/fixtures/benchmarks/Классы/БенчмаркСОбработчикамиСобытий.ostests/fixtures/benchmarks/Классы/БенчмаркиСЭталономИПараметрами.ostests/fixtures/handlers/Модули/ОбработчикиСобытийБенчмарков.ostests/fixtures/paramsSources/Модули/МодульСИсточникамиПараметров.ostests/fixtures/verified-report.jsontests/helpers/Модули/ТестированиеБенчмарков.ostests/ТестыБенчмарков.ostests/ТестыИзолированныхЗапусков.ostests/ТестыИсточниковПараметров.ostests/ТестыКонфигурации.ostests/ТестыСериализаторНастроекЗапускаБенчмарков.ostests/ТестыСериализаторРезультатовБенчмарков.ostests/ТестыСтатистики.ostests/ТестыЭталонов.os
💤 Files with no reviewable changes (2)
- src/BenchmarkOneScript/dto/Классы/РезультатИтерацииБенчмаркаДто.os
- src/BenchmarkOneScript/core/Классы/ЛенивыйДелегат.os
Comment on lines
+255
to
+257
| ИсполняющиеСреды = СтрРазделить(_ИсполняющаяСреда, ","); | ||
| Для Каждого Версия Из ИсполняющиеСреды Цикл | ||
| Конфигурация.ДобавитьИсполняющуюСреду(Версия); |
There was a problem hiding this comment.
Нормализуйте значения --runtime перед добавлением.
СтрРазделить(..., ",") оставляет пробелы, поэтому --runtime "stable, dev" передаст во второй вызов строку " dev". Такая среда уже не совпадет ни с alias, ни с версией.
💡 Вариант исправления
- ИсполняющиеСреды = СтрРазделить(_ИсполняющаяСреда, ",");
- Для Каждого Версия Из ИсполняющиеСреды Цикл
- Конфигурация.ДобавитьИсполняющуюСреду(Версия);
+ ИсполняющиеСреды = СтрРазделить(_ИсполняющаяСреда, ",", Ложь);
+ Для Каждого Версия Из ИсполняющиеСреды Цикл
+ НормализованнаяВерсия = СокрЛП(Версия);
+ Если ЗначениеЗаполнено(НормализованнаяВерсия) Тогда
+ Конфигурация.ДобавитьИсполняющуюСреду(НормализованнаяВерсия);
+ КонецЕсли;
КонецЦикла;📝 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/cmd/Классы/КомандаЗапуска.os` around lines 255 - 257,
Строки, полученные через СтрРазделить(_ИсполняющаяСреда, ","), могут содержать
ведущие/замыкающие пробелы, поэтому перед вызовом
Конфигурация.ДобавитьИсполняющуюСреду(Версия) нужно нормализовать каждую Версия
— удалить пробелы в начале и конце (trim) и пропускать пустые элементы; внесите
это прямо в цикл, где формируется ИсполняющиеСреды и обрабатывается переменная
Версия.
Comment on lines
+11
to
+19
| &Сериализуемое("Baseline") | ||
| &Тип("Булево") | ||
| Перем ЭтоЭталон Экспорт; // Булево | ||
|
|
||
| Процедура ПриСозданииОбъекта() | ||
|
|
||
| Версия = ""; | ||
| Алиас = ""; | ||
| Версия = ""; | ||
| Наименование = ""; | ||
| ЭтоЭталон = Ложь; |
There was a problem hiding this comment.
Keep the serialized runtime contract backward-compatible.
This change replaces the exported runtime payload from an alias-based shape to Baseline, and the updated golden report already drops Runtime.Alias. Any downstream consumer parsing existing JSON reports will break on this PR. Please either preserve Alias through a deprecation window or version/document the report schema change explicitly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/BenchmarkOneScript/core/Классы/ИсполняющаяСредаБенчмарков.os` around
lines 11 - 19, The change renames the serialized runtime shape to "Baseline" and
removes the previously exported alias field, which breaks downstream JSON
consumers expecting Runtime.Alias; restore backward compatibility by keeping the
old serialized name or emitting both fields: keep the new
&Сериализуемое("Baseline") annotation for the new schema but also serialize the
legacy key "Alias" (map the value from the variable ЭтоЭталон) so both
Runtime.Baseline and Runtime.Alias exist during a deprecation window, and update
Процедура ПриСозданииОбъекта() initialization/documentation or add explicit
schema versioning metadata to signal the change.
Comment on lines
+795
to
+804
| Функция ДобавитьИсполняющуюСреду(Версия, Наименование = "", ЭтоЭталон = Ложь) Экспорт | ||
|
|
||
| ИсполняющаяСреда = Новый ИсполняющаяСредаБенчмарков(); | ||
| ИсполняющаяСреда.Версия = Версия; | ||
| ИсполняющаяСреда.Наименование = Наименование; | ||
| ИсполняющаяСреда.ЭтоЭталон = ЭтоЭталон; | ||
|
|
||
| ДанныеСреды = _ВерсииИсполняющейСреды.Добавить(); | ||
| ДанныеСреды.Версия = Версия; | ||
| ДанныеСреды.Наименование = Наименование; | ||
| ДанныеСреды.ЭтоЭталон = ЭтоЭталон; | ||
| _ИсполняющиеСреды.Добавить(ИсполняющаяСреда); | ||
|
|
||
| УстановитьМодифицированность(ИмяПоля_ВерсииИсполняющейСреды()); | ||
| УстановитьМодифицированность(ИмяПоля_ИсполняющиеСреды()); |
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 `@src/BenchmarkOneScript/core/Классы/КонфигурацияБенчмарков.os` around lines
795 - 804, В функции ДобавитьИсполняющуюСреду(Версия, Наименование = "",
ЭтоЭталон = Ложь) предотвратите добавление более чем одной эталонной среды:
перед _ИсполняющиеСреды.Добавить(...) при ЭтоЭталон = Истина пройдитесь по
коллекции _ИсполняющиеСреды и если найдена уже существующая запись с полем
ЭтоЭталон = Истина — прервите операцию и вызовите Ошибка("Уже существует
эталонная исполняющая среда") или верните/логируйте соответствующее сообщение;
это гарантирует, что метод ЭталоннаяИсполняющаяСреда() не будет полагаться на
порядок добавления и сохранит инвариант единственной эталонной среды.
src/BenchmarkOneScript/core/Классы/РезультатЗапускаБенчмарков.os
Outdated
Show resolved
Hide resolved
src/BenchmarkOneScript/core/Классы/ЧитательПараметровБенчмарков.os
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (2)
src/BenchmarkOneScript/cmd/Классы/КомандаЗапуска.os (1)
255-257:⚠️ Potential issue | 🟡 MinorSkip empty
--runtimeentries after trimming.
СокрЛП()fixes spaces, butdev,,stableor a trailing comma still adds""as a runtime. That turns a CLI typo into an invalid environment entry downstream.💡 Suggested fix
ИсполняющиеСреды = СтрРазделить(_ИсполняющаяСреда, ",", Ложь); Для Каждого Версия Из ИсполняющиеСреды Цикл - Конфигурация.ДобавитьИсполняющуюСреду(СокрЛП(Версия)); + НормализованнаяВерсия = СокрЛП(Версия); + Если ЗначениеЗаполнено(НормализованнаяВерсия) Тогда + Конфигурация.ДобавитьИсполняющуюСреду(НормализованнаяВерсия); + КонецЕсли; КонецЦикла;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/cmd/Классы/КомандаЗапуска.os` around lines 255 - 257, При разборе _ИсполняющаяСреда используем СтрРазделить и затем в цикле по ИсполняющиеСреды пропускаем пустые значения после применения СокрЛП — то есть в блоке, где проходит Для Каждого Версия Из ИсполняющиеСреды Цикл, заменить прямой вызов Конфигурация.ДобавитьИсполняющуюСреду(СокрЛП(Версия)) на проверку: вычислить trimmed = СокрЛП(Версия) и только если trimmed <> "" вызвать Конфигурация.ДобавитьИсполняющуюСреду(trimmed), чтобы игнорировать пустые элементы от последовательностей типа "dev,,stable" или завершающей запятой.src/BenchmarkOneScript/core/Классы/ЗапускательБенчмарков.os (1)
141-159:⚠️ Potential issue | 🔴 CriticalВозобновлению все еще нужен пропуск кейсов до
ЗапуститьКейс().Если
_Чекпоинтуже указывает на более поздний кейс, этот цикл все равно зайдет в предыдущие кейсы, потому что восстановление учитывается только на уровне итераций. В холодном запуске это снова создает чекпоинт для неправильного кейса и может зациклить прогресс.🛠️ Минимальный фикс по месту
Процедура ЗапуститьКейсыБенчмарков(ИдентификаторыКейсов, РезультатЗапуска) @@ Для Каждого Кейс Из Кейсы Цикл + + Если ТребуетсяПропуститьКейс(Кейс) Тогда + Продолжить; + КонецЕсли; ПроверитьИСоздатьЧекпоинт(Кейс); Если _НовыйЧекпоинт <> Неопределено Тогда Возврат; КонецЕсли;Функция ТребуетсяПропуститьКейс(Кейс) Если _Чекпоинт = Неопределено Тогда Возврат Ложь; КонецЕсли; Если _БылоВыполнениеИтерации Тогда Возврат Ложь; КонецЕсли; Возврат _Чекпоинт.ИдентификаторКейса <> Кейс.Идентификатор(); КонецФункции🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/Классы/ЗапускательБенчмарков.os` around lines 141 - 159, Цикл в процедуре ЗапуститьКейсыБенчмарков может обрабатывать кейсы раньше текущего чекпоинта; добавьте проверку пропуска перед вызовом ЗапуститьКейс: реализовать функцию ТребуетсяПропуститьКейс(Кейс) которая проверяет _Чекпоинт и флаг _БылоВыполнениеИтерации и возвращает true если текущий Кейс уже пройден (например _Чекпоинт.ИдентификаторКейса <> Кейс.Идентификатор()), и вызвать её в цикле сразу после ПроверитьИСоздатьЧекпоинт — при true пропустить итерацию (не вызывать ЗапуститьКейс) чтобы не создавать некорректный новый чекпоинт и не зациклить прогресс.
🤖 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 233-248: Currently ДобавитьОшибку deduplicates purely on the final
string (ТекстОшибки) which collapses distinct serialization errors that share
identical parameter name/type; update the code so the dedupe key includes the
error context instead of only the formatted text: either augment ТекстОшибки to
embed the source context (e.g., benchmark/case/global parameter name or
identifier) before calling ДобавитьОшибку, or change ДобавитьОшибку to accept a
structured payload (e.g., parameters: Параметр.Имя(), ТипЗнч(...),
ИсточникПараметров) and deduplicate using a composite key (Имя+Тип+Источник)
while still storing/returning the human-readable message; keep _НайденныеОшибки
as a map/set keyed by that composite identifier (or store objects) so distinct
contexts are not collapsed.
In `@src/BenchmarkOneScript/core/Классы/ГенераторКейсовБенчмарков.os`:
- Around line 156-165: Функция ПолучитьСписокИсполняющихСред не должна
модифицировать внутренний массив _Конфигурация.ИсполняющиеСреды() добавлением
Неопределено в него; вместо этого создайте и верните новый массив (скопию
значений ВерсииСреды) и, если он пуст, добавьте в эту новую коллекцию
единственный явный экземпляр дефолтной исполняющей среды (например новый
объект/структуру дефолтной среды) — не мутируйте оригинал; изменяйте логику
вокруг переменных ВерсииСреды и ВыполнятьВТекущейВерсии в функции
ПолучитьСписокИсполняющихСред, чтобы работать с копией и возвращать её.
In `@src/BenchmarkOneScript/core/Классы/РезультатЗапускаБенчмарков.os`:
- Around line 142-160: Функция ПодходитВКачествеЭталона сейчас сравнивает только
Метод при заданной ВерсииЭталоннойСреды и только Категорию иначе, что не
соответствует тому, как потом восстанавливают дескриптор в
НайтиЭталонДляРезультата (по ТипКласса()+Метод()). Измените проверки так, чтобы
для совпадения кейса всегда использовался единый ключ идентичности: сравнивайте
и ТипКласса() и Метод() (например, добавить переменную ИдентичностьСовпадает =
ДескрипторКандидата.ТипКласса() = ДескрипторТекущего.ТипКласса() И
ДескрипторКандидата.Метод() = ДескрипторТекущего.Метод()), затем при
ЭталоннаяСредаЗадана возвращайте ИдентичностьСовпадает И
ВерсияЭталоннойСредыСовпадает, а иначе возвращайте ЭтоЭталон И
КатегорияСовпадает И ИдентичностьСовпадает И ВерсияСредыРезультатаСовпадает.
- Around line 179-200: Родительский ПослеДесериализации в
РезультатЗапускаБенчмарков сейчас вызывает ВосстановитьСсылкиНаДескрипторы и
сразу обращается к Запуск.Кейс, что небезопасно, т.к. не гарантируется, что
дочерний РезультатЗапускаКейса.ПослеДесериализации уже выполнился; исправьте
это, явно восстановив состояние дочерних объектов до доступа: внутри
ВосстановитьСсылкиНаДескрипторы пройдите по Запуски и для каждого вызовите явный
метод восстановления дочернего объекта (например, вызовите у каждого Запуск
метод ВосстановитьСсылкиНаДескриптор или вызвать его ПослеДесериализации), либо
вынесите общую логику восстановления дескриптора в публичный метод на
РезультатЗапускаКейса и вызывать его здесь перед чтением Запуск.Кейс; затем
безопасно выполнить УстановитьДескрипторБенчмарка с найденным
ДескрипторИзКоллекции.
In `@src/BenchmarkOneScript/core/Классы/ЧитательПараметровБенчмарков.os`:
- Around line 104-115: В функции НормализоватьЗначение и в ветвлении внутри
СоздатьНаборДляНесколькихПараметров добавьте проверку на тип
"ФиксированныйМассив" так же, как для "Массив" (т.е. не оборачивать в Новый
Массив и считать его коллекцией), заменив/расширив существующее условие
ТипЗначения = Тип("Массив") Или ... на включение Тип("ФиксированныйМассив"), и
обновите соответствующие ветки в СоздатьНаборДляНесколькихПараметров, чтобы
обрабатывать ФиксированныйМассив как корректный набор параметров вместо
генерации исключения.
In `@tests/ТестыИзолированныхЗапусков.os`:
- Around line 268-270: Remove the stray top-level invocation of the test
function ТестДолжен_ЗапуститьОдинБенчмаркОднойИтерациейБезПараметровСПамятью()
so it does not execute at module load time; either delete that standalone call
or move it into the proper test registration block (&Тест / test suite) so the
test runs only under the test lifecycle and reporting. Ensure no other
standalone calls remain and that the actual test function definition stays
intact.
In `@tests/ТестыКонфигурации.os`:
- Around line 873-875: Remove the top-level invocation of the test procedure to
avoid running it at module load time: delete the standalone call
"ТестДолжен_ВыброситьИсключениеПриГенерацииКейсовСНеизвестнымИдентификатором();"
so the procedure is only executed by the test runner (via its &Тест marker or
test harness). Leave the procedure definition intact and ensure it has the
proper test attribute (&Тест) if required so the test runner will invoke
ТестДолжен_ВыброситьИсключениеПриГенерацииКейсовСНеизвестнымИдентификатором when
running tests.
In `@tests/ТестыЭталонов.os`:
- Around line 122-126: Tests add runtimes but rely on prior global state; call
ТестированиеБенчмарков.ПодготовитьСреду("dev,stable") before invoking
Бенчмаркинг.Запустить so the "dev" and "stable" environments are explicitly
prepared. Specifically, insert a call to
ТестированиеБенчмарков.ПодготовитьСреду("dev,stable") immediately after the
Конфигурация.ДобавитьИсполняющуюСреду("dev")/("stable", ...) lines (around where
Конфигурация.ДобавитьИсполняющуюСреду and Бенчмаркинг.Запустить are used), and
make the same change for the other occurrence (the block at the later occurrence
currently at 152-156).
---
Duplicate comments:
In `@src/BenchmarkOneScript/cmd/Классы/КомандаЗапуска.os`:
- Around line 255-257: При разборе _ИсполняющаяСреда используем СтрРазделить и
затем в цикле по ИсполняющиеСреды пропускаем пустые значения после применения
СокрЛП — то есть в блоке, где проходит Для Каждого Версия Из ИсполняющиеСреды
Цикл, заменить прямой вызов
Конфигурация.ДобавитьИсполняющуюСреду(СокрЛП(Версия)) на проверку: вычислить
trimmed = СокрЛП(Версия) и только если trimmed <> "" вызвать
Конфигурация.ДобавитьИсполняющуюСреду(trimmed), чтобы игнорировать пустые
элементы от последовательностей типа "dev,,stable" или завершающей запятой.
In `@src/BenchmarkOneScript/core/Классы/ЗапускательБенчмарков.os`:
- Around line 141-159: Цикл в процедуре ЗапуститьКейсыБенчмарков может
обрабатывать кейсы раньше текущего чекпоинта; добавьте проверку пропуска перед
вызовом ЗапуститьКейс: реализовать функцию ТребуетсяПропуститьКейс(Кейс) которая
проверяет _Чекпоинт и флаг _БылоВыполнениеИтерации и возвращает true если
текущий Кейс уже пройден (например _Чекпоинт.ИдентификаторКейса <>
Кейс.Идентификатор()), и вызвать её в цикле сразу после
ПроверитьИСоздатьЧекпоинт — при true пропустить итерацию (не вызывать
ЗапуститьКейс) чтобы не создавать некорректный новый чекпоинт и не зациклить
прогресс.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6668c288-9fc5-4d1a-bcc3-8891a573eac9
📒 Files selected for processing (14)
docs/ОбработчикиСобытий.mdsrc/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/Классы/ЧитательПараметровБенчмарков.ostests/ТестыБенчмарков.ostests/ТестыИзолированныхЗапусков.ostests/ТестыКонфигурации.ostests/ТестыЭталонов.os
Comment on lines
+233
to
+248
| ТекстОшибки = СтрШаблон( | ||
| "Параметр <%1> имеет непримитивный тип <%2> и не может быть сериализован. " | ||
| + "Используйте источники параметров (&ИсточникПараметров) для передачи значений составных типов.", | ||
| Параметр.Имя(), | ||
| ТипЗнч(Параметр.Значение()) | ||
| ); | ||
|
|
||
| ДобавитьОшибку(ТекстОшибки); | ||
|
|
||
| КонецПроцедуры | ||
|
|
||
| Процедура ДобавитьОшибку(ТекстОшибки) | ||
|
|
||
| Если _НайденныеОшибки.Найти(ТекстОшибки) = Неопределено Тогда | ||
| _НайденныеОшибки.Добавить(ТекстОшибки); | ||
| КонецЕсли; |
There was a problem hiding this comment.
Не схлопывайте разные ошибки по одинаковому тексту.
Сейчас разные нарушения сериализации с одинаковыми Имя() и типом превратятся в одну запись, потому что дедупликация идет по готовой строке. Так как этот текст потом выбрасывается как сообщение исключения, пользователь может исправить одно место и только на следующем прогоне увидеть второе. Лучше добавлять в сообщение контекст источника (бенчмарк/кейс/общий параметр) или хранить ошибки структурно, а не по финальному тексту.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/BenchmarkOneScript/core/Классы/ВалидаторЗапускаБенчмарков.os` around
lines 233 - 248, Currently ДобавитьОшибку deduplicates purely on the final
string (ТекстОшибки) which collapses distinct serialization errors that share
identical parameter name/type; update the code so the dedupe key includes the
error context instead of only the formatted text: either augment ТекстОшибки to
embed the source context (e.g., benchmark/case/global parameter name or
identifier) before calling ДобавитьОшибку, or change ДобавитьОшибку to accept a
structured payload (e.g., parameters: Параметр.Имя(), ТипЗнч(...),
ИсточникПараметров) and deduplicate using a composite key (Имя+Тип+Источник)
while still storing/returning the human-readable message; keep _НайденныеОшибки
as a map/set keyed by that composite identifier (or store objects) so distinct
contexts are not collapsed.
Comment on lines
+156
to
+165
| Функция ПолучитьСписокИсполняющихСред() | ||
|
|
||
| ВерсииСреды = _Конфигурация.ИсполняющиеСреды(); | ||
| ВыполнятьВТекущейВерсии = ВерсииСреды.Количество() = 0; | ||
|
|
||
| Если ВыполнятьВТекущейВерсии Тогда | ||
| ВерсииСреды.Добавить(); | ||
| КонецЕсли; | ||
|
|
||
| Возврат ВерсииСреды; |
There was a problem hiding this comment.
Не подмешивайте Неопределено в общий список исполняющих сред.
Этот helper возвращает внутренний массив конфигурации и при пустом списке добавляет в него Неопределено. Дальше каждый КейсБенчмарка создает свою ИсполняющаяСредаБенчмарков, поэтому в ИзолированныйЗапускательБенчмарков кейсы текущей среды перестают батчиться вместе при группировке по Кейс.ИсполняющаяСреда(). Здесь лучше вернуть отдельный массив и переиспользовать один явный экземпляр среды по умолчанию.
💡 Один из вариантов исправления
Функция ПолучитьСписокИсполняющихСред()
- ВерсииСреды = _Конфигурация.ИсполняющиеСреды();
- ВыполнятьВТекущейВерсии = ВерсииСреды.Количество() = 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
156 - 165, Функция ПолучитьСписокИсполняющихСред не должна модифицировать
внутренний массив _Конфигурация.ИсполняющиеСреды() добавлением Неопределено в
него; вместо этого создайте и верните новый массив (скопию значений ВерсииСреды)
и, если он пуст, добавьте в эту новую коллекцию единственный явный экземпляр
дефолтной исполняющей среды (например новый объект/структуру дефолтной среды) —
не мутируйте оригинал; изменяйте логику вокруг переменных ВерсииСреды и
ВыполнятьВТекущейВерсии в функции ПолучитьСписокИсполняющихСред, чтобы работать
с копией и возвращать её.
Comment on lines
+142
to
+160
| Функция ПодходитВКачествеЭталона(КандидатВЭталоны, ТекущийРезультат, ВерсияЭталоннойСреды) | ||
|
|
||
| ДескрипторКандидата = КандидатВЭталоны.Кейс.ДескрипторБенчмарка(); | ||
| СредаКандидата = КандидатВЭталоны.Кейс.ИсполняющаяСреда(); | ||
| ДескрипторТекущего = ТекущийРезультат.Кейс.ДескрипторБенчмарка(); | ||
| СредаТекущего = ТекущийРезультат.Кейс.ИсполняющаяСреда(); | ||
|
|
||
| ЭтоЭталон = ДескрипторКандидата.ЭтоЭталон(); | ||
| ИмяМетодаСовпадает = ДескрипторКандидата.Метод() = ДескрипторТекущего.Метод(); | ||
| КатегорияСовпадает = ДескрипторКандидата.Категория() = ДескрипторТекущего.Категория(); | ||
|
|
||
| ЭталоннаяСредаЗадана = ЗначениеЗаполнено(ВерсияЭталоннойСреды); | ||
| ВерсияЭталоннойСредыСовпадает = СредаКандидата.Версия = ВерсияЭталоннойСреды; | ||
| ВерсияСредыРезультатаСовпадает = СредаКандидата.Версия = СредаТекущего.Версия; | ||
|
|
||
| Если ЭталоннаяСредаЗадана Тогда | ||
| Возврат ИмяМетодаСовпадает И ВерсияЭталоннойСредыСовпадает; | ||
| Иначе | ||
| Возврат ЭтоЭталон И КатегорияСовпадает И ВерсияСредыРезультатаСовпадает; |
There was a problem hiding this comment.
Используйте здесь тот же ключ идентичности кейса, что и ниже при восстановлении дескриптора.
На Lines 157-160 при заданной эталонной среде сравнивается только Метод, а без нее — только Категория. При этом на Lines 194-197 дескриптор восстанавливается по ТипКласса() + Метод(). Из-за этой асимметрии НайтиЭталонДляРезультата() может привязать чужой эталон, если в наборе есть кейсы с одинаковым методом или одинаковой категорией.
🔧 Proposed fix
ЭтоЭталон = ДескрипторКандидата.ЭтоЭталон();
+ ТипКлассаСовпадает = ДескрипторКандидата.ТипКласса() = ДескрипторТекущего.ТипКласса();
ИмяМетодаСовпадает = ДескрипторКандидата.Метод() = ДескрипторТекущего.Метод();
- КатегорияСовпадает = ДескрипторКандидата.Категория() = ДескрипторТекущего.Категория();
ЭталоннаяСредаЗадана = ЗначениеЗаполнено(ВерсияЭталоннойСреды);
ВерсияЭталоннойСредыСовпадает = СредаКандидата.Версия = ВерсияЭталоннойСреды;
ВерсияСредыРезультатаСовпадает = СредаКандидата.Версия = СредаТекущего.Версия;
Если ЭталоннаяСредаЗадана Тогда
- Возврат ИмяМетодаСовпадает И ВерсияЭталоннойСредыСовпадает;
+ Возврат ТипКлассаСовпадает
+ И ИмяМетодаСовпадает
+ И ВерсияЭталоннойСредыСовпадает;
Иначе
- Возврат ЭтоЭталон И КатегорияСовпадает И ВерсияСредыРезультатаСовпадает;
+ Возврат ЭтоЭталон
+ И ТипКлассаСовпадает
+ И ИмяМетодаСовпадает
+ И ВерсияСредыРезультатаСовпадает;
КонецЕсли;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/BenchmarkOneScript/core/Классы/РезультатЗапускаБенчмарков.os` around
lines 142 - 160, Функция ПодходитВКачествеЭталона сейчас сравнивает только Метод
при заданной ВерсииЭталоннойСреды и только Категорию иначе, что не соответствует
тому, как потом восстанавливают дескриптор в НайтиЭталонДляРезультата (по
ТипКласса()+Метод()). Измените проверки так, чтобы для совпадения кейса всегда
использовался единый ключ идентичности: сравнивайте и ТипКласса() и Метод()
(например, добавить переменную ИдентичностьСовпадает =
ДескрипторКандидата.ТипКласса() = ДескрипторТекущего.ТипКласса() И
ДескрипторКандидата.Метод() = ДескрипторТекущего.Метод()), затем при
ЭталоннаяСредаЗадана возвращайте ИдентичностьСовпадает И
ВерсияЭталоннойСредыСовпадает, а иначе возвращайте ЭтоЭталон И
КатегорияСовпадает И ИдентичностьСовпадает И ВерсияСредыРезультатаСовпадает.
Comment on lines
179
to
+200
| Процедура ПослеДесериализации() Экспорт | ||
|
|
||
| ПереопределитьДескрипторыВЗапусках(); | ||
| ОпределитьЭталоны(); | ||
| ОбновитьСтатистику(); | ||
| ПостроитьОтчет(); | ||
| ВосстановитьСсылкиНаДескрипторы(); | ||
| УстановитьСвязиСЭталонами(); | ||
| ПересчитатьСтатистику(); | ||
| СформироватьОтчет(); | ||
|
|
||
| КонецПроцедуры | ||
|
|
||
| Процедура ПереопределитьДескрипторыВЗапусках() | ||
| Процедура ВосстановитьСсылкиНаДескрипторы() | ||
|
|
||
| Для Каждого Запуск Из Запуски Цикл | ||
| Дескриптор = Запуск.ДескрипторБенчмарка; | ||
| НайденныйДескриптор = ДескрипторыБенчмарков.НайтиПоТипуИИмени(Дескриптор.ТипКласса(), Дескриптор.Метод()); | ||
| Если Не НайденныйДескриптор = Неопределено Тогда | ||
| Запуск.ДескрипторБенчмарка = НайденныйДескриптор; | ||
|
|
||
| ДескрипторИзКейса = Запуск.Кейс.ДескрипторБенчмарка(); | ||
|
|
||
| ДескрипторИзКоллекции = ДескрипторыБенчмарков.НайтиПоТипуИИмени( | ||
| ДескрипторИзКейса.ТипКласса(), | ||
| ДескрипторИзКейса.Метод() | ||
| ); | ||
|
|
||
| Если ДескрипторИзКоллекции <> Неопределено Тогда | ||
| Запуск.Кейс.УстановитьДескрипторБенчмарка(ДескрипторИзКоллекции); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd 'РезультатЗапускаКейса\.os$' src --exec sh -c '
file="$1"
echo "== $file =="
rg -n -C3 "Перем Кейс Экспорт|&Несериализуемое|ПослеДесериализации|ПриСозданииОбъекта|УстановитьКейс|Кейс\s*=|Процедура .*Кейс|Функция .*Кейс" "$file"
' sh {}Repository: Stivo182/BenchmarkOneScript
Length of output: 908
🏁 Script executed:
sed -n '99,130p' src/BenchmarkOneScript/core/Классы/РезультатЗапускаКейса.osRepository: Stivo182/BenchmarkOneScript
Length of output: 947
🌐 Web query:
1C Enterprise @ПослеДесериализации hook execution order inheritance child parent
💡 Result:
Официального описания порядка вызова хука @ПослеДесериализации при наследовании (сценарий «есть реализация в родителе и в потомке») в публичной документации, доступной через поиск, я не нашёл — в доках встречается лишь общее описание механизма аннотаций @... и самой иерархии типов, без регламента порядка вызовов таких хуков. [1], [2]
Что делать на практике (надёжный вариант):
- Не полагаться на порядок “сам по себе”.
- Явно вызывать логику предка из потомка (если язык/платформа это позволяет для ваших типов), либо вынести общую логику в общий метод и вызывать его из обоих мест в нужном порядке.
- Проверить экспериментом в вашей версии платформы (это самый быстрый способ получить “истину” для конкретного рантайма):
- в родителе и потомке сделайте
@ПослеДесериализациии выведите в лог уникальные сообщения, - десериализуйте объект потомка (JSON/XML/внутренний формат — в зависимости от вашего кейса),
- посмотрите фактический порядок.
- в родителе и потомке сделайте
Если напишете, о какой именно технологии/рантайме речь (обычный BSL 1С:Предприятие 8.x vs 1С:Предприятие.Элемент vs 1С:Исполнитель) и чем именно десериализуете (JSON, XDTO/XML, ЗначениеИзСтрокиВнутр, и т.п.), я сузю ответ до вашего стека и дам точный минимальный пример теста под него.
Sources:
[1] https://1cmycloud.com/console/help/executor/docs/topics/annotations/
[2] https://1cmycloud.com/console/help/executor/6.0/docs/topics/terms/type-hierarchy/
🌐 Web query:
1С ПослеДесериализации порядок выполнения в иерархии классов
💡 Result:
В стандартной платформе 1С:Предприятие (BSL) у механизма «встроенной» сериализации/десериализации (например, ЗначениеВФайл/ЗначениеИзФайла, ХранилищеЗначения и т. п.) нет штатного “хука”/события ПослеДесериализации, и, соответственно, нет и “порядка вызовов по иерархии классов” (платформа просто восстанавливает значение). [1]
Если вы про JSON (ЗаписатьJSON/ПрочитатьJSON), то платформа предлагает не ПослеДесериализации, а механизм пользовательской функции восстановления, которую вы явно передаёте в ПрочитатьJSON() — и уже в этой функции вы сами реализуете нужную логику (включая “вызвать базовый, потом наследника” или наоборот). Автоматического обхода “иерархии” платформа не делает. [2]
Если вы используете стороннюю/самописную ООП-надстройку с “классами”, то порядок выполнения ПослеДесериализации определяется реализацией этой надстройки; обычно (и безопаснее всего) его фиксируют вручную: в методе наследника явно вызвать логику базового класса в нужном месте.
Источники:
[1] Документация 1С (ITS): особенности и механизмы сериализации значений в платформе
[2] Документация 1С (ITS): пользовательские функции преобразования/восстановления для JSON (ЗаписатьJSON/ПрочитатьJSON)
🏁 Script executed:
head -20 src/BenchmarkOneScript/core/Классы/РезультатЗапускаБенчмарков.osRepository: Stivo182/BenchmarkOneScript
Length of output: 702
🏁 Script executed:
rg -n "Процедура ПослеДесериализации|Суперкласс|Наследование" src/BenchmarkOneScript/core/Классы/РезультатЗапускаБенчмарков.osRepository: Stivo182/BenchmarkOneScript
Length of output: 114
🏁 Script executed:
rg -B5 "class РезультатЗапускаБенчмарков|РезультатЗапускаБенчмарков.*:" src/BenchmarkOneScript/core/Классы/РезультатЗапускаБенчмарков.osRepository: Stivo182/BenchmarkOneScript
Length of output: 53
🏁 Script executed:
rg -B5 "class РезультатЗапускаКейса|РезультатЗапускаКейса.*:" src/BenchmarkOneScript/core/Классы/РезультатЗапускаКейса.osRepository: Stivo182/BenchmarkOneScript
Length of output: 53
🏁 Script executed:
head -50 src/BenchmarkOneScript/core/Классы/РезультатЗапускаБенчмарков.osRepository: Stivo182/BenchmarkOneScript
Length of output: 1727
🏁 Script executed:
head -50 src/BenchmarkOneScript/core/Классы/РезультатЗапускаКейса.osRepository: Stivo182/BenchmarkOneScript
Length of output: 1210
🏁 Script executed:
sed -n '179,210p' src/BenchmarkOneScript/core/Классы/РезультатЗапускаБенчмарков.osRepository: Stivo182/BenchmarkOneScript
Length of output: 713
The code assumes undocumented behavior: that child object deserialization hooks execute before the parent's ПослеДесериализации().
On line 192, Запуск.Кейс is accessed in ВосстановитьСсылкиНаДескрипторы(), which is called from РезультатЗапускаБенчмарков.ПослеДесериализации(). The Запуски array contains РезультатЗапускаКейса objects, each with a non-serializable Кейс field that is restored in РезультатЗапускаКейса.ПослеДесериализации().
However, there is no official 1С documentation guaranteeing the execution order of @ПослеДесериализации hooks in object hierarchies or during deserialization of nested/collection objects. If child hooks execute after the parent's hook, the code will attempt to dereference an Undefined value.
To make this safe, explicitly invoke the child restoration before accessing the field—either by calling the child's hook method from the parent, extracting common logic to a separate phase, or documenting and testing this assumption for your specific platform version.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/BenchmarkOneScript/core/Классы/РезультатЗапускаБенчмарков.os` around
lines 179 - 200, Родительский ПослеДесериализации в РезультатЗапускаБенчмарков
сейчас вызывает ВосстановитьСсылкиНаДескрипторы и сразу обращается к
Запуск.Кейс, что небезопасно, т.к. не гарантируется, что дочерний
РезультатЗапускаКейса.ПослеДесериализации уже выполнился; исправьте это, явно
восстановив состояние дочерних объектов до доступа: внутри
ВосстановитьСсылкиНаДескрипторы пройдите по Запуски и для каждого вызовите явный
метод восстановления дочернего объекта (например, вызовите у каждого Запуск
метод ВосстановитьСсылкиНаДескриптор или вызвать его ПослеДесериализации), либо
вынесите общую логику восстановления дескриптора в публичный метод на
РезультатЗапускаКейса и вызывать его здесь перед чтением Запуск.Кейс; затем
безопасно выполнить УстановитьДескрипторБенчмарка с найденным
ДескрипторИзКоллекции.
Comment on lines
+104
to
+115
| Функция НормализоватьЗначение(Значение) | ||
|
|
||
| ТипЗначения = ТипЗнч(Значение); | ||
|
|
||
| Если ТипЗначения = Тип("Массив") Или ТипЗначения = Тип("ТаблицаЗначений") Тогда | ||
| Возврат Значение; | ||
| КонецЕсли; | ||
|
|
||
| МассивЗначений = Новый Массив(); | ||
| МассивЗначений.Добавить(Значение); | ||
|
|
||
| Возврат МассивЗначений; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Ищем, используются ли ФиксированныеМассивы в источниках параметров или фикстурах.
rg -n -C2 'ФиксированныйМассив'Repository: Stivo182/BenchmarkOneScript
Length of output: 5746
🏁 Script executed:
# First, let's read the file to see the context around lines 104-115 and 151-166
wc -l "src/BenchmarkOneScript/core/Классы/ЧитательПараметровБенчмарков.os"Repository: Stivo182/BenchmarkOneScript
Length of output: 141
🏁 Script executed:
# Read the relevant sections
sed -n '100,170p' "src/BenchmarkOneScript/core/Классы/ЧитательПараметровБенчмарков.os"Repository: Stivo182/BenchmarkOneScript
Length of output: 1964
🏁 Script executed:
# Look for СоздатьНаборДляНесколькихПараметров function
rg -n "СоздатьНаборДляНесколькихПараметров" "src/BenchmarkOneScript/core/Классы/ЧитательПараметровБенчмарков.os" -A 30Repository: Stivo182/BenchmarkOneScript
Length of output: 1520
🏁 Script executed:
# Find where ИсточникиПараметров is used
rg -n "ИсточникиПараметров" "src/BenchmarkOneScript/core/Классы/ЧитательПараметровБенчмарков.os" -A 5 -B 2Repository: Stivo182/BenchmarkOneScript
Length of output: 53
🏁 Script executed:
# Look for methods that call or execute sources
rg -n "Выполнить|Execute" "src/BenchmarkOneScript/core/Классы/ЧитательПараметровБенчмарков.os" -B 3 -A 3Repository: Stivo182/BenchmarkOneScript
Length of output: 558
Add support for ФиксированныйМассив in parameter normalization and branching.
НормализоватьЗначение() only preserves Массив and ТаблицаЗначений, treating ФиксированныйМассив as a scalar, which causes it to be unnecessarily wrapped in Новый Массив(). This breaks multi-parameter scenarios in СоздатьНаборДляНесколькихПараметров() since it also lacks handling for ФиксированныйМассив and will throw an "unsupported type" exception.
Add support for ФиксированныйМассив in both locations, treating it as a first-class collection type alongside Массив.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/BenchmarkOneScript/core/Классы/ЧитательПараметровБенчмарков.os` around
lines 104 - 115, В функции НормализоватьЗначение и в ветвлении внутри
СоздатьНаборДляНесколькихПараметров добавьте проверку на тип
"ФиксированныйМассив" так же, как для "Массив" (т.е. не оборачивать в Новый
Массив и считать его коллекцией), заменив/расширив существующее условие
ТипЗначения = Тип("Массив") Или ... на включение Тип("ФиксированныйМассив"), и
обновите соответствующие ветки в СоздатьНаборДляНесколькихПараметров, чтобы
обрабатывать ФиксированныйМассив как корректный набор параметров вместо
генерации исключения.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
tests/ТестыЭталонов.os (1)
148-158:⚠️ Potential issue | 🟠 MajorExplicitly prepare
dev/stablein this multi-runtime test.This test adds both runtimes but never calls
ТестированиеБенчмарков.ПодготовитьСреду("dev,stable"), so isolated runs still depend on leaked global state from earlier tests.💡 Suggested fix
Конфигурация = Новый КонфигурацияБенчмарков(Тип); ТестированиеБенчмарков.НастроитьКонфигурациюПодТесты(Конфигурация); Конфигурация.ДобавитьИсполняющуюСреду("dev"); Конфигурация.ДобавитьИсполняющуюСреду("stable"); + ТестированиеБенчмарков.ПодготовитьСреду("dev,stable"); // Действие Результат = Бенчмаркинг.Запустить(Тип, Конфигурация);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыЭталонов.os` around lines 148 - 158, The test adds two runtimes via Конфигурация.ДобавитьИсполняющуюСреду("dev") and ("stable") but never explicitly initializes them, leaving isolated runs dependent on leaked global state; before calling Бенчмаркинг.Запустить(Тип, Конфигурация) invoke ТестированиеБенчмарков.ПодготовитьСреду for both runtimes (e.g., call ТестированиеБенчмарков.ПодготовитьСреду("dev,stable") or two calls for "dev" and "stable") so the Конфигурация is fully prepared and the test is self-contained.
🤖 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 448-452: The loop in ИсполнительБенчмарков that assigns
ПараметрыМетода[ИменаПараметров.Найти(Параметр.Имя())] = Параметр.Значение()
must validate the result of ИменаПараметров.Найти before using it as an index;
change the loop to capture the lookup into a variable (e.g., idx =
ИменаПараметров.Найти(Параметр.Имя())), check if idx = Неопределено (or
otherwise invalid) and handle it—either skip the assignment, log/collect a clear
error, or throw a descriptive exception—so you never index ПараметрыМетода with
an undefined value.
- Around line 253-254: The current computation of Погрешность uses
(ВремяПредыдущейОперации - ВремяОперации) / (ВремяОперации +
ВремяПредыдущейОперации) which can divide by zero when both ВремяОперации and
ВремяПредыдущейОперации are 0; change the code around the Погрешность assignment
to first compute the denominator = (ВремяОперации + ВремяПредыдущейОперации) and
if denominator = 0 set Погрешность = 0 (or another safe sentinel), otherwise
compute the fraction, then apply the absolute-value step currently done with
?(Погрешность < 0, -Погрешность, Погрешность); this fixes the division-by-zero
for the Погрешность calculation.
In `@src/BenchmarkOneScript/core/Модули/Бенчмаркинг.os`:
- Around line 23-33: Комментарий к структуре РезультатЗапускаКейса устарел: он
всё ещё упоминает "ДескрипторБенчмарка", тогда как реальная модель описана в
классе РезультатЗапускаКейса (файл
src/BenchmarkOneScript/core/Классы/РезультатЗапускаКейса.os) и представляет поля
через Кейс / ИдентификаторКейса / Тип / Метод / Категория и т.д.; исправьте блок
комментария в Модули/Бенчмаркинг.os, заменив упоминание ДескрипторБенчмарка на
точный перечень полей из класса РезультатЗапускаКейса (и при необходимости
обновите остальные вложенные поля, чтобы комментарий полностью соответствовал
контракту класса).
In `@tests/ТестыКонфигурации.os`:
- Line 799: Rename the misspelled variable ДескрпиторыБенчмарков to
ДескрипторыБенчмарков in the collection initialization (the assignment using
Новый КоллекцияДескрипторовБенчмарков(Тип)) and update every subsequent
reference/usage in this file to the corrected name so all occurrences match
exactly.
In `@tests/ТестыЭталонов.os`:
- Around line 263-267: Add an existence check for the filtered run before
accessing its .Эталон: after calling
ProcessорыКоллекций.ИзКоллекции(Результат.Запуски).Фильтровать(...).ПолучитьПервый()
(the variable СтрокаБезЭталона), assert that СтрокаБезЭталона is not null/пустое
using the same pattern as scenarios 2/3 (e.g. Ожидаем.Что(СтрокаБезЭталона,
"...").Не_().Заполнено()), and only then perform the assertion on
СтрокаБезЭталона.Эталон to avoid null dereference.
---
Duplicate comments:
In `@tests/ТестыЭталонов.os`:
- Around line 148-158: The test adds two runtimes via
Конфигурация.ДобавитьИсполняющуюСреду("dev") and ("stable") but never explicitly
initializes them, leaving isolated runs dependent on leaked global state; before
calling Бенчмаркинг.Запустить(Тип, Конфигурация) invoke
ТестированиеБенчмарков.ПодготовитьСреду for both runtimes (e.g., call
ТестированиеБенчмарков.ПодготовитьСреду("dev,stable") or two calls for "dev" and
"stable") so the Конфигурация is fully prepared and the test is self-contained.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 531746c9-2732-4c7a-9fb3-1004c12e8e45
📒 Files selected for processing (16)
README.mddocs/CLI.mddocs/БыстрыйСтарт.mddocs/ЗапускБенчмарков.mddocs/Параметризация.mddocs/СтратегииЗапуска.mdsrc/BenchmarkOneScript/cmd/Классы/КомандаЗапуска.ossrc/BenchmarkOneScript/core/Классы/ГенераторКейсовБенчмарков.ossrc/BenchmarkOneScript/core/Классы/ЗапускательБенчмарков.ossrc/BenchmarkOneScript/core/Классы/ИзолированныйЗапускательБенчмарков.ossrc/BenchmarkOneScript/core/Классы/ИсполнительБенчмарков.ossrc/BenchmarkOneScript/core/Классы/СценарийЗапускаБенчмарка.ossrc/BenchmarkOneScript/core/Модули/Бенчмаркинг.ostests/ТестыИзолированныхЗапусков.ostests/ТестыКонфигурации.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/cmd/Классы/КомандаЗапуска.os
Comment on lines
+253
to
+254
| Погрешность = (ВремяПредыдущейОперации - ВремяОперации) / (ВремяОперации + ВремяПредыдущейОперации); | ||
| Погрешность = ?(Погрешность < 0, -Погрешность, Погрешность); |
There was a problem hiding this comment.
Potential division by zero when calculating error margin.
When both ВремяОперации and ВремяПредыдущейОперации are zero (e.g., on the first iteration), the denominator becomes zero, which could cause a runtime error.
🛡️ Proposed fix to guard against zero denominator
- Погрешность = (ВремяПредыдущейОперации - ВремяОперации) / (ВремяОперации + ВремяПредыдущейОперации);
+ СуммаВремени = ВремяОперации + ВремяПредыдущейОперации;
+ Если СуммаВремени > 0 Тогда
+ Погрешность = (ВремяПредыдущейОперации - ВремяОперации) / СуммаВремени;
+ Иначе
+ Погрешность = 0;
+ КонецЕсли;
- Погрешность = ?(Погрешность < 0, -Погрешность, Погрешность);
+ Погрешность = ?(Погрешность < 0, -Погрешность, Погрешность);📝 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
| Погрешность = (ВремяПредыдущейОперации - ВремяОперации) / (ВремяОперации + ВремяПредыдущейОперации); | |
| Погрешность = ?(Погрешность < 0, -Погрешность, Погрешность); | |
| СуммаВремени = ВремяОперации + ВремяПредыдущейОперации; | |
| Если СуммаВремени > 0 Тогда | |
| Погрешность = (ВремяПредыдущейОперации - ВремяОперации) / СуммаВремени; | |
| Иначе | |
| Погрешность = 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 253
- 254, The current computation of Погрешность uses (ВремяПредыдущейОперации -
ВремяОперации) / (ВремяОперации + ВремяПредыдущейОперации) which can divide by
zero when both ВремяОперации and ВремяПредыдущейОперации are 0; change the code
around the Погрешность assignment to first compute the denominator =
(ВремяОперации + ВремяПредыдущейОперации) and if denominator = 0 set Погрешность
= 0 (or another safe sentinel), otherwise compute the fraction, then apply the
absolute-value step currently done with ?(Погрешность < 0, -Погрешность,
Погрешность); this fixes the division-by-zero for the Погрешность calculation.
Comment on lines
+448
to
+452
| Для Каждого Параметр Из Параметры Цикл | ||
| Если Параметр.ЭтоПараметрМетода() Тогда | ||
| ПараметрыМетода[ИменаПараметров.Найти(Параметр.Имя())] = Параметр.Значение(); | ||
| КонецЕсли; | ||
| КонецЦикла; |
There was a problem hiding this comment.
Missing validation for parameter name lookup result.
If ИменаПараметров.Найти(Параметр.Имя()) returns Неопределено (parameter name not found in descriptor), using it as an array index will cause a runtime error.
🛡️ Proposed fix to validate parameter lookup
Для Каждого Параметр Из Параметры Цикл
Если Параметр.ЭтоПараметрМетода() Тогда
- ПараметрыМетода[ИменаПараметров.Найти(Параметр.Имя())] = Параметр.Значение();
+ ИндексПараметра = ИменаПараметров.Найти(Параметр.Имя());
+ Если ИндексПараметра <> Неопределено Тогда
+ ПараметрыМетода[ИндексПараметра] = Параметр.Значение();
+ КонецЕсли;
КонецЕсли;
КонецЦикла;📝 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 448
- 452, The loop in ИсполнительБенчмарков that assigns
ПараметрыМетода[ИменаПараметров.Найти(Параметр.Имя())] = Параметр.Значение()
must validate the result of ИменаПараметров.Найти before using it as an index;
change the loop to capture the lookup into a variable (e.g., idx =
ИменаПараметров.Найти(Параметр.Имя())), check if idx = Неопределено (or
otherwise invalid) and handle it—either skip the assignment, log/collect a clear
error, or throw a descriptive exception—so you never index ПараметрыМетода with
an undefined value.
Comment on lines
+23
to
+33
| // * Запуски - Массив из РезультатЗапускаКейса: | ||
| // ** ДескрипторБенчмарка - ДескрипторБенчмарка | ||
| // ** Параметры - Массив из ПараметрБенчмарка | ||
| // ** Эталон - РезультатЗапускаБенчмаркаДто, Неопределено | ||
| // ** ЭтоЭталон - Булево | ||
| // ** Статистика - СтатистикаБенчмарка | ||
| // ** ИсполняющаяСреда - ИсполняющаяСредаБенчмарковДто: | ||
| // *** Версия - Строка | ||
| // *** Алиас - Строка | ||
| // *** Наименование - Строка | ||
| // ** Замеры - Массив из РезультатИтерацииБенчмаркаДто: | ||
| // *** Этап - см. ЭтапыБенчмарка | ||
| // *** НомерИтерации - Число | ||
| // *** КоличествоОпераций - Число | ||
| // *** Наносекунд - Число - Наносекунд за итерацию | ||
| // *** НаносекундЗаОперацию - Число - Наносекунд за операцию | ||
| // *** ВыделяемаяПамять - Число - Байт выделяемой памяти за итерацию | ||
| // ** Параметры - Массив из ПараметрБенчмарка | ||
| // ** Эталон - РезультатЗапускаКейса, Неопределено | ||
| // ** ЭтоЭталон - Булево | ||
| // ** Статистика - СтатистикаБенчмарка | ||
| // ** ИсполняющаяСреда - ИсполняющаяСредаБенчмарков: | ||
| // *** Версия - Строка | ||
| // *** Алиас - Строка | ||
| // *** Наименование - Строка | ||
| // ** Замеры - Массив из РезультатИтерацииКейса: |
There was a problem hiding this comment.
РезультатЗапускаКейса здесь описан по старой модели.
На Line 24 всё ещё указан ДескрипторБенчмарка, хотя src/BenchmarkOneScript/core/Классы/РезультатЗапускаКейса.os:1-44 уже описывает кейс через Кейс / ИдентификаторКейса / Тип / Метод / Категория и т.д. В таком виде публичный комментарий расходится с фактическим контрактом и будет вводить в заблуждение потребителей API.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/BenchmarkOneScript/core/Модули/Бенчмаркинг.os` around lines 23 - 33,
Комментарий к структуре РезультатЗапускаКейса устарел: он всё ещё упоминает
"ДескрипторБенчмарка", тогда как реальная модель описана в классе
РезультатЗапускаКейса (файл
src/BenchmarkOneScript/core/Классы/РезультатЗапускаКейса.os) и представляет поля
через Кейс / ИдентификаторКейса / Тип / Метод / Категория и т.д.; исправьте блок
комментария в Модули/Бенчмаркинг.os, заменив упоминание ДескрипторБенчмарка на
точный перечень полей из класса РезультатЗапускаКейса (и при необходимости
обновите остальные вложенные поля, чтобы комментарий полностью соответствовал
контракту класса).
|
|
||
| // Подготовка | ||
| Тип = Тип("ПустойБенчмарк"); | ||
| ДескрпиторыБенчмарков = Новый КоллекцияДескрипторовБенчмарков(Тип); |
There was a problem hiding this comment.
Typo in variable name.
The variable ДескрпиторыБенчмарков has a typo — should be ДескрипторыБенчмарков (missing "и").
✏️ Proposed fix
- ДескрпиторыБенчмарков = Новый КоллекцияДескрипторовБенчмарков(Тип);
+ ДескрипторыБенчмарков = Новый КоллекцияДескрипторовБенчмарков(Тип);Also update line 809:
- ГенераторКейсов = Новый ГенераторКейсовБенчмарков(ДескрпиторыБенчмарков, Конфигурация);
+ ГенераторКейсов = Новый ГенераторКейсовБенчмарков(ДескрипторыБенчмарков, Конфигурация);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/ТестыКонфигурации.os` at line 799, Rename the misspelled variable
ДескрпиторыБенчмарков to ДескрипторыБенчмарков in the collection initialization
(the assignment using Новый КоллекцияДескрипторовБенчмарков(Тип)) and update
every subsequent reference/usage in this file to the corrected name so all
occurrences match exactly.
Comment on lines
+263
to
+267
| СтрокаБезЭталона = ПроцессорыКоллекций.ИзКоллекции(Результат.Запуски) | ||
| .Фильтровать("Элемент -> Элемент.Кейс.ДескрипторБенчмарка().Метод() = ""БенчмаркВ""") | ||
| .ПолучитьПервый(); | ||
|
|
||
| Ожидаем.Что(СтрокаБезЭталона.Эталон, "Эталон не должен быть найден при несовпадении имен параметров").Не_().Заполнено(); |
There was a problem hiding this comment.
Assert the filtered run exists before dereferencing .Эталон.
If the selector stops matching, Line 267 fails with a null dereference instead of a useful assertion. Scenarios 2 and 3 already guard against this; scenario 1 should do the same.
💡 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 `@tests/ТестыЭталонов.os` around lines 263 - 267, Add an existence check for
the filtered run before accessing its .Эталон: after calling
ProcessорыКоллекций.ИзКоллекции(Результат.Запуски).Фильтровать(...).ПолучитьПервый()
(the variable СтрокаБезЭталона), assert that СтрокаБезЭталона is not null/пустое
using the same pattern as scenarios 2/3 (e.g. Ожидаем.Что(СтрокаБезЭталона,
"...").Не_().Заполнено()), and only then perform the assertion on
СтрокаБезЭталона.Эталон to avoid null dereference.
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
Documentation
New Features
Refactor