fix: учитывать локальный oscript.cfg при запуске бенчмарков - #58
Conversation
- находить конфигурации OneScript рядом с файлами бенчмарков - подключать локальные зависимости при первичной загрузке - применять конфигурацию конкретного бенчмарка при изолированном запуске - сохранять конфигурацию для файлов без аннотации Бенчмарк
|
Warning Review limit reached
Next review available in: 18 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe change adds local ChangesConfigured benchmark execution
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant ConfigLocator
participant ConfigManager
participant RestartLauncher
participant OneScript
CLI->>ConfigLocator: Resolve benchmark-adjacent oscript.cfg files
ConfigLocator-->>CLI: Ordered configuration paths
CLI->>RestartLauncher: Apply configurations
RestartLauncher->>ConfigManager: Read and merge sources
ConfigManager-->>RestartLauncher: Configuration parameters
RestartLauncher->>OneScript: Start main.os with OSCRIPT_CONFIG
OneScript-->>RestartLauncher: Return exit code
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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: 5
🧹 Nitpick comments (4)
tests/ТестыЛокатораФайловКонфигурацииOneScript.os (1)
283-290: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
ЭтоWindows()implementation — deferred to consolidated comment.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/ТестыЛокатораФайловКонфигурацииOneScript.os` around lines 283 - 290, Remove the duplicate ЭтоWindows() implementation from this location and reuse the existing consolidated implementation instead, updating any local references if needed while preserving the Windows platform detection behavior.src/BenchmarkOneScript/cmd/Классы/ЛокаторФайловКонфигурацииOneScript.os (1)
82-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
ЭтоWindows()implementation — deferred to consolidated comment.This is the same platform-detection logic duplicated across this file,
РаботаСПутямиФайловойСистемы.os, and a test file; see consolidated comment.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BenchmarkOneScript/cmd/Классы/ЛокаторФайловКонфигурацииOneScript.os` around lines 82 - 89, Remove the duplicate ЭтоWindows() implementation from this file and reuse the existing shared platform-detection helper used by РаботаСПутямиФайловойСистемы.os and the tests. Update callers as needed so Windows detection remains unchanged while maintaining a single implementation.src/BenchmarkOneScript/core/common/Модули/РаботаСПутямиФайловойСистемы.os (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
ЭтоWindows()implemented identically in three places.The same platform-detection logic is copy-pasted across a common utility module, a CLI locator class, and a test file, risking drift if the platform check ever needs adjustment.
src/BenchmarkOneScript/core/common/Модули/РаботаСПутямиФайловойСистемы.os#L85-88: export this helper (it's currently private) so it can be the single source of truth.src/BenchmarkOneScript/cmd/Классы/ЛокаторФайловКонфигурацииOneScript.os#L82-89: remove the local copy and call the exported common helper instead.tests/ТестыЛокатораФайловКонфигурацииOneScript.os#L283-290: remove the local copy and call the exported common helper instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BenchmarkOneScript/core/common/Модули/РаботаСПутямиФайловойСистемы.os` at line 1, Export ЭтоWindows() from РаботаСПутямиФайловойСистемы.os, then remove the duplicated platform-detection implementations in ЛокаторФайловКонфигурацииOneScript and ТестыЛокатораФайловКонфигурацииOneScript, replacing their uses with the shared exported helper.tests/ТестыМенеджераПараметровКонфигурацииOneScript.os (1)
49-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider
&Параметры(...)for these input-variant tests.Both tests exercise two input variants (
Строка/ФиксированныйМассивandНеопределено/"") against the same expected outcome, which fits the project convention of preferring parameterized tests when only input data changes.As per coding guidelines: "Prefer parameterized tests through
&Параметры(...)when only input data and expectations change." fortests/**/*.os.Also applies to: 103-123
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/ТестыМенеджераПараметровКонфигурацииOneScript.os` around lines 49 - 77, Объедините тесты вариантов входных данных в параметризованный тест с директивой &Параметры(...): используйте один сценарий для проверки МенеджерПараметровКонфигурацииOneScript с входами Строка/ФиксированныйМассив и Неопределено/"". Сохраните общую проверку ожидаемых параметров, изменяя только входные данные и соответствующие ожидания.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/BenchmarkOneScript/core/common/Классы/РазрешательВходныхФайловБенчмарков.os`:
- Around line 1-2: Remove the `#Использовать` collectionos dependency and update
the sorting in the benchmark input file resolver to avoid Массивы.Сортировать
and collectionos. Replace that call with a manual sort or another available
built-in approach while preserving direct-order sorting of ПолныеИменаФайлов.
In `@src/BenchmarkOneScript/core/common/Модули/РаботаСПутямиФайловойСистемы.os`:
- Line 1: Canonicalize temporary-directory paths at creation time in the test
helper by resolving symlinks once, so ТекущийКаталог(),
НайтиПолноеИмяФайлаСценария, and СформироватьИсточникиКонфигурации compare the
same physical path representation. Preserve downstream path construction and
test assertions; do not patch the macOS-specific expectations.
In `@tests/ТестыЗапускателяПриложенияСКонфигурациейOneScript.os`:
- Around line 159-163: Replace the parent-process assertion on
ПолучитьПеременнуюСреды("BENCHOS_OSCRIPT_CONFIG_APPLIED") with an assertion
against observable child-process output or another propagated result that
confirms the restart marker was applied exactly once. Preserve the existing
library-load count check and ensure the test fails when the child restart logic
does not execute correctly.
In `@tests/ТестыЛокатораФайловКонфигурацииOneScript.os`:
- Around line 192-206: Обновите проверку в
ТестДолжен_РазрешитьЯвныйФайлБезРасширенияOS так, чтобы ожидаемый и фактический
пути сравнивались в одной канонической форме на macOS, включая нормализацию
префикса /private. Сохраните проверку единственного найденного файла и
используйте существующий механизм нормализации путей, если он доступен.
In `@tests/ТестыМенеджераПараметровКонфигурацииOneScript.os`:
- Around line 228-293: Update the path expectations in
ТестДолжен_ДобавитьКаталогиБиблиотекИзВсехКонфигурацийПоПриоритету to
canonicalize every expected path consistently with the paths produced by
МенеджерПараметровКонфигурацииOneScript and ТекущийКаталог(), including all six
lib.additional entries and lib.system. Reuse the existing path-normalization
helper or established canonicalization flow rather than comparing raw ПолныйПуть
results.
---
Nitpick comments:
In `@src/BenchmarkOneScript/cmd/Классы/ЛокаторФайловКонфигурацииOneScript.os`:
- Around line 82-89: Remove the duplicate ЭтоWindows() implementation from this
file and reuse the existing shared platform-detection helper used by
РаботаСПутямиФайловойСистемы.os and the tests. Update callers as needed so
Windows detection remains unchanged while maintaining a single implementation.
In `@src/BenchmarkOneScript/core/common/Модули/РаботаСПутямиФайловойСистемы.os`:
- Line 1: Export ЭтоWindows() from РаботаСПутямиФайловойСистемы.os, then remove
the duplicated platform-detection implementations in
ЛокаторФайловКонфигурацииOneScript and ТестыЛокатораФайловКонфигурацииOneScript,
replacing their uses with the shared exported helper.
In `@tests/ТестыЛокатораФайловКонфигурацииOneScript.os`:
- Around line 283-290: Remove the duplicate ЭтоWindows() implementation from
this location and reuse the existing consolidated implementation instead,
updating any local references if needed while preserving the Windows platform
detection behavior.
In `@tests/ТестыМенеджераПараметровКонфигурацииOneScript.os`:
- Around line 49-77: Объедините тесты вариантов входных данных в
параметризованный тест с директивой &Параметры(...): используйте один сценарий
для проверки МенеджерПараметровКонфигурацииOneScript с входами
Строка/ФиксированныйМассив и Неопределено/"". Сохраните общую проверку ожидаемых
параметров, изменяя только входные данные и соответствующие ожидания.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ffac55c5-4dfb-4512-af4d-cf325b0f3450
📒 Files selected for processing (25)
.gitignoredocs/CLI.mddocs/ЗапускБенчмарков.mdsrc/BenchmarkOneScript/cmd/main.ossrc/BenchmarkOneScript/cmd/Классы/ЗапускательПриложенияСКонфигурациейOneScript.ossrc/BenchmarkOneScript/cmd/Классы/КомандаЗапуска.ossrc/BenchmarkOneScript/cmd/Классы/ЛокаторФайловКонфигурацииOneScript.ossrc/BenchmarkOneScript/core/common/Классы/РазрешательВходныхФайловБенчмарков.ossrc/BenchmarkOneScript/core/common/Модули/РаботаСПутямиФайловойСистемы.ossrc/BenchmarkOneScript/core/common/Модули/ЭкранированиеАргументовКоманднойСтроки.ossrc/BenchmarkOneScript/core/config/Классы/МенеджерПараметровКонфигурацииOneScript.ossrc/BenchmarkOneScript/core/running/Классы/ЗапускательФайловБенчмарков.ossrc/BenchmarkOneScript/core/running/Классы/МенеджерПроцессовБенчмарков.ossrc/BenchmarkOneScript/core/running/Классы/ПодготовительФайлаБенчмарка.ossrc/BenchmarkOneScript/core/serialization/jason/Классы/ДесериализаторJsonBench.ostests/helpers/Модули/ТестированиеБенчмарков.ostests/helpers/Модули/ТестированиеПроцессов.ostests/ТестыЗапускателяПриложенияСКонфигурациейOneScript.ostests/ТестыЗапускателяФайловБенчмарков.ostests/ТестыЛокатораФайловКонфигурацииOneScript.ostests/ТестыМенеджераПараметровКонфигурацииOneScript.ostests/ТестыМенеджераПроцессовБенчмарков.ostests/ТестыПодготовителяФайлаБенчмарка.ostests/ТестыРазрешателяВходныхФайловБенчмарков.ostests/ТестыСериализатораРезультатовБенчмарков.os
| #Использовать fs | ||
| #Использовать collectionos |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift
#Использовать collectionos fails to load in CI — build-breaking.
Pipeline logs show this exact import failing across multiple macOS runners: OneScript failed to load required library: 'collectionos', breaking ТестДолжен_ИгнорироватьКонфигурациюСтартовогоСценария. This dependency isn't resolvable in the test environment.
🐛 Suggested direction
`#Использовать` fs
-#Использовать collectionosReplace Массивы.Сортировать(ПолныеИменаФайлов, СравнениеЗначений.ПрямойПорядок()) (line 81) with a manual sort or a built-in alternative that doesn't require this external library.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/BenchmarkOneScript/core/common/Классы/РазрешательВходныхФайловБенчмарков.os`
around lines 1 - 2, Remove the `#Использовать` collectionos dependency and update
the sorting in the benchmark input file resolver to avoid Массивы.Сортировать
and collectionos. Replace that call with a manual sort or another available
built-in approach while preserving direct-order sorting of ПолныеИменаФайлов.
Sources: Learnings, Pipeline failures
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)
src/BenchmarkOneScript/core/config/Классы/МенеджерПараметровКонфигурацииOneScript.os (1)
52-72: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winDocstring priority order is now stale.
Порядок приоритета файлов конфигурации: 1. Дополнительные файлы конфигурации в переданном порядке 2. Текущий каталог 3. Каталог программы — but
СформироватьИсточникиКонфигурацииnow inserts the startup-script directory's config between "additional files" and "current directory" (Lines 163-167), which isn't reflected here.📝 Suggested doc update
// Порядок приоритета файлов конфигурации: // 1. Дополнительные файлы конфигурации в переданном порядке -// 2. Текущий каталог -// 3. Каталог программы +// 2. Каталог стартового сценария +// 3. Текущий каталог +// 4. Каталог программыAs per coding guidelines, "Document public methods, constructors, and other program interface elements with comments when touching or adding them."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BenchmarkOneScript/core/config/Классы/МенеджерПараметровКонфигурацииOneScript.os` around lines 52 - 72, Обновите комментарий перед экспортной функцией Прочитать, чтобы порядок источников отражал фактическую последовательность СформироватьИсточникиКонфигурации: после дополнительных файлов укажите конфигурацию из каталога стартового скрипта, затем текущий каталог и каталог программы.Source: Coding guidelines
♻️ Duplicate comments (2)
tests/ТестыМенеджераПараметровКонфигурацииOneScript.os (1)
195-266: 🎯 Functional Correctness | 🔴 CriticalUnresolved: macOS symlink path mismatch in this test — previously flagged.
This test (and the new "current directory" assertions added at Lines 254-263) still compares against
ПолныйПуть()(348-352,Новый Файл(...).ПолноеИмя, no symlink resolution) while production resolution (ФС.НормализоватьПуть) canonicalizes symlinks on macOS (e.g./var→/private/var), per prior CI failure logs on this exact test. Since the comparison helper wasn't changed, this failure mode remains for the newly-added assertions too.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/ТестыМенеджераПараметровКонфигурацииOneScript.os` around lines 195 - 266, Update the path expectations in ТестДолжен_ДобавитьКаталогиБиблиотекИзВсехКонфигурацийПоПриоритету, including the current-configuration assertions, to use the same symlink-resolving normalization as production’s ФС.НормализоватьПуть instead of ПолныйПуть. Ensure every compared expected path is canonicalized so macOS `/var` and `/private/var` representations match.Source: Pipeline failures
tests/ТестыЗапускателяПриложенияСКонфигурациейOneScript.os (1)
71-143: 🎯 Functional Correctness | 🟡 MinorLine 141's assertion still doesn't test the restart guard — previously flagged.
The line 141 check reads
BENCHOS_OSCRIPT_CONFIG_APPLIEDin the parent test process, but per the upstream contract Функция СоздатьИЗапуститьПроцесс... ПеременныеСредыПроцесса.Вставить("BENCHOS_OSCRIPT_CONFIG_APPLIED", "1") only mutates the spawned child's own environment map, not the parent's. Since the parent's copy was blanked at Line 84 and never touched again, this assertion passes regardless of whether the child's "restart once" logic is correct. The library-load count check at Lines 137-140 is the real regression guard and is unaffected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/ТестыЗапускателяПриложенияСКонфигурациейOneScript.os` around lines 71 - 143, Remove the ineffective parent-process assertion on BENCHOS_OSCRIPT_CONFIG_APPLIED from ТестДолжен_ОдинРазПерезапуститьRunСКонфигурациямиOneScript, since the child process environment cannot update it. Keep the existing КоличествоЗагрузокБиблиотеки assertion as the regression check for the single-restart behavior.
🧹 Nitpick comments (2)
tests/ТестыМенеджераПараметровКонфигурацииOneScript.os (1)
269-317: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winOrphaned helper functions — no test exercises the new startup-script config source.
ТекстСтартовогоСценария,ВыполнитьСтартовыйСценарий, andПутьКИсполняемомуФайлуOneScript(plus the globals at Lines 10, 20-32 they depend on) are defined but never called by any test in this file. See consolidated comment.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/ТестыМенеджераПараметровКонфигурацииOneScript.os` around lines 269 - 317, Remove the unused helper functions ТекстСтартовогоСценария, ВыполнитьСтартовыйСценарий, and ПутьКИсполняемомуФайлуOneScript, along with the dependent globals, since no test invokes the startup-script configuration source. Ensure the remaining tests and setup no longer reference those removed symbols.src/BenchmarkOneScript/core/config/Классы/МенеджерПараметровКонфигурацииOneScript.os (1)
153-180: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNew "startup script" config source lacks dedicated test coverage.
The addition at Lines 163-167 introduces a new, higher-priority config source resolved from
СтартовыйСценарий().Каталог. This can't be exercised by directly instantiating the manager in-process (the startup script is fixed for the whole process), and no test in the accompanying test file spawns a subprocess to verify it — see the consolidated note.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BenchmarkOneScript/core/config/Классы/МенеджерПараметровКонфигурацииOneScript.os` around lines 153 - 180, Добавьте отдельный тест для СформироватьИсточникиКонфигурации, запускающий BenchmarkOneScript в дочернем процессе с заданным стартовым сценарием и конфигурацией в его каталоге. Проверьте, что источник из СтартовыйСценарий().Каталог имеет приоритет и его параметры применяются раньше источников текущего каталога и каталога программы.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@src/BenchmarkOneScript/core/config/Классы/МенеджерПараметровКонфигурацииOneScript.os`:
- Around line 52-72: Обновите комментарий перед экспортной функцией Прочитать,
чтобы порядок источников отражал фактическую последовательность
СформироватьИсточникиКонфигурации: после дополнительных файлов укажите
конфигурацию из каталога стартового скрипта, затем текущий каталог и каталог
программы.
---
Duplicate comments:
In `@tests/ТестыЗапускателяПриложенияСКонфигурациейOneScript.os`:
- Around line 71-143: Remove the ineffective parent-process assertion on
BENCHOS_OSCRIPT_CONFIG_APPLIED from
ТестДолжен_ОдинРазПерезапуститьRunСКонфигурациямиOneScript, since the child
process environment cannot update it. Keep the existing
КоличествоЗагрузокБиблиотеки assertion as the regression check for the
single-restart behavior.
In `@tests/ТестыМенеджераПараметровКонфигурацииOneScript.os`:
- Around line 195-266: Update the path expectations in
ТестДолжен_ДобавитьКаталогиБиблиотекИзВсехКонфигурацийПоПриоритету, including
the current-configuration assertions, to use the same symlink-resolving
normalization as production’s ФС.НормализоватьПуть instead of ПолныйПуть. Ensure
every compared expected path is canonicalized so macOS `/var` and `/private/var`
representations match.
---
Nitpick comments:
In
`@src/BenchmarkOneScript/core/config/Классы/МенеджерПараметровКонфигурацииOneScript.os`:
- Around line 153-180: Добавьте отдельный тест для
СформироватьИсточникиКонфигурации, запускающий BenchmarkOneScript в дочернем
процессе с заданным стартовым сценарием и конфигурацией в его каталоге.
Проверьте, что источник из СтартовыйСценарий().Каталог имеет приоритет и его
параметры применяются раньше источников текущего каталога и каталога программы.
In `@tests/ТестыМенеджераПараметровКонфигурацииOneScript.os`:
- Around line 269-317: Remove the unused helper functions
ТекстСтартовогоСценария, ВыполнитьСтартовыйСценарий, and
ПутьКИсполняемомуФайлуOneScript, along with the dependent globals, since no test
invokes the startup-script configuration source. Ensure the remaining tests and
setup no longer reference those removed symbols.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a00be6ee-0389-44f8-ae42-0a7ffbfe46d5
📒 Files selected for processing (3)
src/BenchmarkOneScript/core/config/Классы/МенеджерПараметровКонфигурацииOneScript.ostests/ТестыЗапускателяПриложенияСКонфигурациейOneScript.ostests/ТестыМенеджераПараметровКонфигурацииOneScript.os
|
1 participant
Проблема
При запуске
benchos runиз каталога, отличного от каталога бенчмарков,OneScript не учитывал
oscript.cfg, расположенный рядом с бенчмарком.Из-за этого не подключались локальные зависимости, указанные через
lib.systemилиlib.additional. Особенно это проявлялось, если пакетотсутствовал в глобальном каталоге библиотек.
Решение
oscript.cfgрядом с переданными файлами и найденнымибенчмарками.
перезапускается с собранными путями библиотек.
lib.systemиlib.additional.глобальной.
рядом с конкретным бенчмарком.
oscript.cfg.&Бенчмарклокальная конфигурация переноситсяво временный каталог вместе с подготовленным файлом.
командной строки вынесена в отдельные компоненты.
Summary by CodeRabbit
oscript.cfg, including local library dependencies, and reuse the configuration on execution..osextension are now resolved automatically.oscript.cfgand local dependency rules..gitignoreto ignore Git worktrees.