feat: Добавлен запуск plain .os-файлов с общим отчетом#50
Merged
Conversation
При смешанном запуске plain .os-файлы объединяются в один отчет, а бенчмарки с аннотацией &Бенчмарк публикуются отдельно.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughRefactors CLI run flow to delegate file discovery and grouped execution to a new ЗапускательФайловБенчмарков; adds merging of results for plain Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as КомандаЗапуска
participant FLauncher as ЗапускательФайловБенчмарков
participant Preparer as ПодготовительФайлаБенчмарка
participant Launcher as ЗапускательБенчмарков
participant Publisher as ПубликаторРезультатаЗапускаБенчмарков
participant Exporter as ЭкспортерРезультатовБенчмарков
CLI->>FLauncher: Запустить(массив путей, настройки)
FLauncher->>FLauncher: Обнаружить и сгруппировать файлы
loop для каждой группы
FLauncher->>Preparer: СодержитБенчмарки(файл)?
Preparer-->>FLauncher: boolean
alt подготовленные файлы
FLauncher->>Launcher: Новый(Источник, Конфигурация, Репортер)
FLauncher->>Launcher: Запустить()
else обычные алгоритмы
FLauncher->>Preparer: ОписаниеАлгоритмаИзФайла(...)
Preparer-->>FLauncher: ОписаниеАлгоритма
FLauncher->>Preparer: СоздатьВременныйФайлБенчмарка(Описание)
Preparer-->>FLauncher: temp .os
FLauncher->>Launcher: Запустить()
end
Launcher-->>FLauncher: РезультатЗапуска
alt требуется объединение
FLauncher->>FLauncher: Объединить дескрипторы и запуски
FLauncher->>Publisher: Опубликовать(mergedResult)
else
FLauncher->>Publisher: Опубликовать(РезультатЗапуска)
end
end
Publisher->>Publisher: ВывестиРезультатЗапуска()
Publisher->>Exporter: Новый ЭкспортерРезультатовБенчмарков(репортер)
Publisher->>Exporter: Экспортер.Записать(результат)
Exporter->>Exporter: ИмяФайлаОтчета(результат)
FLauncher-->>CLI: Завершено
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: 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/exporters/Классы/ЭкспортерРезультатовБенчмарков.os (1)
10-20:⚠️ Potential issue | 🟠 MajorОчистка артефактов теперь не убирает старые per-benchmark отчеты.
После перехода на
benchmarks-report.*вы удаляете только текущую маску. Если в каталоге уже лежит старыйАлгоритмБезАннотации-report.jsonот предыдущего запуска, mixed-run оставит его рядом с новым общим отчетом, и он будет выглядеть как артефакт текущего прогона.Also applies to: 38-61
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/exporters/Классы/ЭкспортерРезультатовБенчмарков.os` around lines 10 - 20, Текущий код формирует МаскаФайлов = СтрШаблон("%1.*", ИмяФайла) и вызывает ФС.УдалитьФайлы(Каталог, МаскаФайлов), из‑за чего остаются старые per‑benchmark файлы; исправьте это, расширив логику удаления: после вычисления ИмяФайла (ИмяФайлаОтчета) удаляйте не только МаскаФайлов, но и шаблоны для per‑benchmark отчётов (например все "*-report.*" или конкретный шаблон вида "%1-*-report.*"/"benchmarks-report.*"), т.е. дополните вызовы ФС.УдалитьФайлы(Каталог, ...) или добавьте цикл по массиву масок и удаляйте каждую маску (ссылки: ИмяФайлаОтчета, МаскаФайлов, ФС.УдалитьФайлы).
🤖 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/running/Классы/ЗапускательФайловБенчмарков.os`:
- Around line 122-155: В каталожной ветке заменить логику добавления найденных
*.os: вместо прямого создания новой группы для каждого Файл.ПолноеИмя, получить
ПолноеИмяФайла = ПолноеИмяФайлаСценария(Файл.ПолноеИмя), пропустить если
ДобавленныеИсходныеФайлы.Содержит(ПолноеИмяФайла), добавить в
ДобавленныеИсходныеФайлы и затем точно так же, как в ветке для одиночного файла,
проверять _ПодготовительФайлаБенчмарка.СодержитБенчмарки(ПолноеИмяФайла) и либо
ГруппыФайлов.Добавить(НоваяГруппаФайловБенчмарков(МассивФайлов(ПолноеИмяФайла),
Ложь)), либо создать/использовать ГруппаОбычныхАлгоритмов и добавить
подготовленный файл через
_ПодготовительФайлаБенчмарка.Подготовить(ПолноеИмяФайла) в её Файлы; используйте
те же идентификаторы (НайтиФайлы, ДобавленныеИсходныеФайлы, ГруппыФайлов,
ГруппаОбычныхАлгоритмов) чтобы сохранить поведение для plain .os.
In `@tests/ТестыЗапускателяФайловБенчмарков.os`:
- Around line 97-125: В тесте
ТестДолжен_ЗаписатьПредупреждениеЕслиНеНайденыФайлыБенчмарков текущая очистка
логгера выполняется только до проверок и не защищена от падений; оберните
получение сообщений и все последующие проверочные утверждения в блок, который
гарантированно выполняет восстановление логгера (например Использовать
конструкцию Попытка ... Кроме ... КонецПопытки с финальной очисткой или отдельно
Попытка/Исключение/КонецПопытки для действия и затем Всегда/КонецВсегда для
удаления аппендера и восстановления уровня), чтобы методы
Лог.УдалитьСпособВывода и Лог.УстановитьУровень всегда вызывались даже при
падении проверок; используйте Аппендер.ПолучитьСообщения только внутри
защищённого блока перед утверждениями.
---
Outside diff comments:
In
`@src/BenchmarkOneScript/core/exporters/Классы/ЭкспортерРезультатовБенчмарков.os`:
- Around line 10-20: Текущий код формирует МаскаФайлов = СтрШаблон("%1.*",
ИмяФайла) и вызывает ФС.УдалитьФайлы(Каталог, МаскаФайлов), из‑за чего остаются
старые per‑benchmark файлы; исправьте это, расширив логику удаления: после
вычисления ИмяФайла (ИмяФайлаОтчета) удаляйте не только МаскаФайлов, но и
шаблоны для per‑benchmark отчётов (например все "*-report.*" или конкретный
шаблон вида "%1-*-report.*"/"benchmarks-report.*"), т.е. дополните вызовы
ФС.УдалитьФайлы(Каталог, ...) или добавьте цикл по массиву масок и удаляйте
каждую маску (ссылки: ИмяФайлаОтчета, МаскаФайлов, ФС.УдалитьФайлы).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 52cb74d4-2ec7-4330-be82-74e480e79401
📒 Files selected for processing (15)
docs/CLI.mddocs/ЗапускБенчмарков.mdsrc/BenchmarkOneScript/cmd/Классы/КомандаЗапуска.ossrc/BenchmarkOneScript/core/exporters/Классы/ЭкспортерРезультатовБенчмарков.ossrc/BenchmarkOneScript/core/reporting/Классы/ПубликаторРезультатаЗапускаБенчмарков.ossrc/BenchmarkOneScript/core/running/Классы/ЗапускательБенчмарков.ossrc/BenchmarkOneScript/core/running/Классы/ЗапускательФайловБенчмарков.ossrc/BenchmarkOneScript/core/running/Классы/ИсполнительБенчмарков.ossrc/BenchmarkOneScript/core/running/Классы/ПодготовительФайлаБенчмарка.ossrc/BenchmarkOneScript/core/running/Классы/РезультатЗапускаБенчмарков.ossrc/BenchmarkOneScript/core/running/Модули/Бенчмаркинг.ostests/fixtures/АлгоритмБезАннотации2.ostests/fixtures/АппендерЛога.ostests/ТестыЗапускателяФайловБенчмарков.ostests/ТестыПодготовителяФайлаБенчмарка.os
src/BenchmarkOneScript/core/running/Классы/ЗапускательФайловБенчмарков.os
Show resolved
Hide resolved
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/BenchmarkOneScript/core/running/Классы/ЗапускательФайловБенчмарков.os (1)
122-135:⚠️ Potential issue | 🟠 MajorФайлы из каталогов обходят ветку подготовки plain
.osи не попадут в общий отчет.В ветке каталога (строки 126-134) каждый найденный
*.osсразу добавляется как отдельная группа сОбъединятьРезультаты = Ложь, без проверкиСодержитБенчмарки(). Приrun <каталог>с plain-скриптами они не объединятся в merged-flow.Также
Файл.ПолноеИмяне нормализуется, что может привести к дубликатам при смешанном вводе каталога и явного файла.Предлагаемое исправление
Для Каждого Файл Из НайденныеФайлы Цикл - Если ДобавленныеИсходныеФайлы.Содержит(Файл.ПолноеИмя) Тогда + ПолноеИмяФайла = ФС.НормализоватьПуть(Файл.ПолноеИмя); + Если ДобавленныеИсходныеФайлы.Содержит(ПолноеИмяФайла) Тогда Продолжить; КонецЕсли; - ДобавленныеИсходныеФайлы.Добавить(Файл.ПолноеИмя); - ГруппыФайлов.Добавить(НоваяГруппаФайловБенчмарков(МассивФайлов(Файл.ПолноеИмя), Ложь)); + ДобавленныеИсходныеФайлы.Добавить(ПолноеИмяФайла); + + Если _ПодготовительФайлаБенчмарка.СодержитБенчмарки(ПолноеИмяФайла) Тогда + ГруппыФайлов.Добавить(НоваяГруппаФайловБенчмарков(МассивФайлов(ПолноеИмяФайла), Ложь)); + Иначе + Если ГруппаОбычныхАлгоритмов = Неопределено Тогда + ГруппаОбычныхАлгоритмов = НоваяГруппаФайловБенчмарков(Новый Массив(), Истина); + ГруппыФайлов.Добавить(ГруппаОбычныхАлгоритмов); + КонецЕсли; + + ГруппаОбычныхАлгоритмов.Файлы.Добавить(_ПодготовительФайлаБенчмарка.Подготовить(ПолноеИмяФайла)); + КонецЕсли; КонецЦикла;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BenchmarkOneScript/core/running/Классы/ЗапускательФайловБенчмарков.os` around lines 122 - 135, В ветке обработки каталогов в классе/файле ЗапускательФайловБенчмарков.os исправьте логику добавления найденных .os: перед созданием группы вызывайте СодержитБенчмарки(Файл.ПолноеИмя) и только если она возвращает true создавайте группу как сейчас; для plain-скриптов (когда СодержитБенчмарки=false) создавайте группу с ОбъединятьРезультаты = Истина (или используйте ту же семантику, что и для merged-flow) чтобы обеспечить объединение результатов; и нормализуйте/канонизируйте путь (например, через существующую утилиту нормализации пути) для Файл.ПолноеИмя перед проверки/добавления в ДобавленныеИсходныеФайлы, чтобы избежать дубликатов при смешанном вводе. Используйте идентификаторы НайденныеФайлы, ДобавленныеИсходныеФайлы, ГруппыФайлов и НоваяГруппаФайловБенчмарков для локализации изменений.
🧹 Nitpick comments (1)
tests/ТестыЗапускателяФайловБенчмарков.os (1)
237-268: Мутация входного массива.Строка 246 добавляет элемент в переданный
МассивФайлов, что является побочным эффектом. Хотя в текущих вызовах это не вызывает проблем (массивы создаются заново черезМассивФайлов()), рекомендуется работать с копией для предсказуемости.Предлагаемое исправление
Функция ВыполнитьСЗахватомЛогаПодготовителя(МассивФайлов) Лог = Логирование.ПолучитьЛог("oscript.lib.benchmark.ПодготовительФайлаБенчмарка"); ПредыдущийУровень = Лог.Уровень(); Аппендер = ЗагрузитьСценарий(ПолныйПутьКФайлу("./tests/fixtures/АппендерЛога.os")); Лог.УстановитьУровень(УровниЛога.Отладка); Лог.ДобавитьСпособВывода(Аппендер, УровниЛога.Отладка); - МассивФайлов.Добавить("ФайлКоторогоНеСуществуетДляТестаЗапускателя.os"); + ЛокальныйМассив = Новый Массив(); + Для Каждого Путь Из МассивФайлов Цикл + ЛокальныйМассив.Добавить(Путь); + КонецЦикла; + ЛокальныйМассив.Добавить("ФайлКоторогоНеСуществуетДляТестаЗапускателя.os"); Попытка - Запускатель = Новый ЗапускательФайловБенчмарков(МассивФайлов); + Запускатель = Новый ЗапускательФайловБенчмарков(ЛокальныйМассив);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/ТестыЗапускателяФайловБенчмарков.os` around lines 237 - 268, Функция ВыполнитьСЗахватомЛогаПодготовителя мутирует входной параметр: строка, где выполняется МассивФайлов.Добавить("ФайлКоторогоНеСуществуетДляТестаЗапускателя.os"), вносит побочный эффект; вместо этого создайте локальную копию входного массива (например, ЛокальныйМассив = Новый Массив(МассивФайлов) или вручную скопируйте элементы в новый Массив) и добавляйте тестовый элемент в эту копию, затем передавайте копию в конструктор Новый ЗапускательФайловБенчмарков; оставьте входной МассивФайлов неизменным.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/BenchmarkOneScript/core/running/Классы/ЗапускательФайловБенчмарков.os`:
- Around line 122-135: В ветке обработки каталогов в классе/файле
ЗапускательФайловБенчмарков.os исправьте логику добавления найденных .os: перед
созданием группы вызывайте СодержитБенчмарки(Файл.ПолноеИмя) и только если она
возвращает true создавайте группу как сейчас; для plain-скриптов (когда
СодержитБенчмарки=false) создавайте группу с ОбъединятьРезультаты = Истина (или
используйте ту же семантику, что и для merged-flow) чтобы обеспечить объединение
результатов; и нормализуйте/канонизируйте путь (например, через существующую
утилиту нормализации пути) для Файл.ПолноеИмя перед проверки/добавления в
ДобавленныеИсходныеФайлы, чтобы избежать дубликатов при смешанном вводе.
Используйте идентификаторы НайденныеФайлы, ДобавленныеИсходныеФайлы,
ГруппыФайлов и НоваяГруппаФайловБенчмарков для локализации изменений.
---
Nitpick comments:
In `@tests/ТестыЗапускателяФайловБенчмарков.os`:
- Around line 237-268: Функция ВыполнитьСЗахватомЛогаПодготовителя мутирует
входной параметр: строка, где выполняется
МассивФайлов.Добавить("ФайлКоторогоНеСуществуетДляТестаЗапускателя.os"), вносит
побочный эффект; вместо этого создайте локальную копию входного массива
(например, ЛокальныйМассив = Новый Массив(МассивФайлов) или вручную скопируйте
элементы в новый Массив) и добавляйте тестовый элемент в эту копию, затем
передавайте копию в конструктор Новый ЗапускательФайловБенчмарков; оставьте
входной МассивФайлов неизменным.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: be3991e8-eae8-4364-9e1d-6abaf20532ab
📒 Files selected for processing (4)
src/BenchmarkOneScript/cmd/Классы/КомандаЗапуска.ossrc/BenchmarkOneScript/core/exporters/Классы/ЭкспортерРезультатовБенчмарков.ossrc/BenchmarkOneScript/core/running/Классы/ЗапускательФайловБенчмарков.ostests/ТестыЗапускателяФайловБенчмарков.os
🚧 Files skipped from review as they are similar to previous changes (2)
- src/BenchmarkOneScript/core/exporters/Классы/ЭкспортерРезультатовБенчмарков.os
- src/BenchmarkOneScript/cmd/Классы/КомандаЗапуска.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.
При смешанном запуске plain .os-файлы объединяются в один отчет, а бенчмарки с аннотацией &Бенчмарк публикуются отдельно.
Summary by CodeRabbit
New Features
Documentation
Tests
Refactor