-
Notifications
You must be signed in to change notification settings - Fork 2
feat: Поддержка загрузки библиотек из каталога, заданного в конфиге и ENV при запуске сервиса #19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…енных окружения при старте приложения
📝 WalkthroughWalkthroughBuild/CI and dependency updates; added a CFG parameter provider; revamped process environment assembly and propagation to spawned processes; CLI option for custom controllers and file-search tweak; tests switched to lazy HttpBin startup and renamed test env var. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Runner
participant CLI as CLI / Launcher
participant HttpBin as HttpBin (manager)
participant Provider as CFG Provider / configor
participant Child as Spawned Process
Test->>HttpBin: request/start (lazy init)
HttpBin->>Provider: ПрочитатьПараметры() / load cfgs
Provider-->>HttpBin: return parameters map
HttpBin->>HttpBin: ПолучитьПараметрыСредыПроцесса() — merge, resolve paths
HttpBin->>Child: spawn process with augmented OSCRIPT_CONFIG env
Child->>Child: init using OSCRIPT_CONFIG
Test->>Child: run tests against spawned process
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cmd/Классы/ЗапускательСервиса.os (1)
34-41: Переименование env var может сломать существующие сценарии: лучше поддержать оба имени хотя бы на один релиз.Proposed fix (fallback)
- Если ПолучитьПеременнуюСреды("HTTPBIN_TESTING_MODE") = "true" Тогда + РежимТестирования = ПолучитьПеременнуюСреды("HTTPBIN_TESTING_MODE"); + Если ПустаяСтрока(РежимТестирования) Тогда + РежимТестирования = ПолучитьПеременнуюСреды("HTTPBIN_IS_TEST_MODE"); + КонецЕсли; + Если НРег(РежимТестирования) = "true" Тогда _НастройкиВебСервера.ЗадержкаПередЧтениемСокета = 400; КонецЕсли;
🤖 Fix all issues with AI agents
In @.github/workflows/test.yml:
- Around line 10-21: The matrix include is incorrectly assigning oscript_version
as an array; change the include block to list explicit scalar entries for each
combination so oscript_version is a string not an array: replace the single
include entry that sets oscript_version: ['stable', 'dev'] and test_engine:
'oneunit' with two separate include items (one with oscript_version: 'stable'
and test_engine: 'oneunit', and one with oscript_version: 'dev' and test_engine:
'oneunit') so the matrix produces separate jobs and passes scalar
oscript_version values to the reusable workflow.
In @src/cmd/Классы/КомандаЗапустить.os:
- Around line 21-36: Опция "routes-handlers" объявлена и связана с переменной
_КастомныеКонтроллеры, но эта переменная нигде не используется, создавая мёртвый
API; исправьте либо: (1) заменить ручной разбор АргументыКоманднойСтроки в месте
запуска на использование значения _КастомныеКонтроллеры (передавать/парсить её в
ту же логику, где подключаются контроллеры), либо (2) явно отметить в коде и
комментариях, что _КастомныеКонтроллеры служит только для генерации help и
удалить/пометить рефактором чтение из АргументыКоманднойСтроки; обновите
соответствующую документацию/комментарий рядом с объявлением переменной и
уберите маркер BSLLS:UnusedLocalVariable, если переменная начнёт реально
использоваться.
🧹 Nitpick comments (1)
src/internal/Классы/ПровайдерПараметровCFG.os (1)
84-115: Парсинг CFG: стоит триммить строку перед проверкой#и обернуть чтение файла в try/catch.Иначе строки вида
" #comment"не будут считаться комментариями, а ошибки чтения (кодировка/доступ) будут “пробивать” наружу без контекста.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/workflows/qa.yml.github/workflows/test.ymlpackagedefsrc/cmd/Классы/ЗапускательСервиса.ossrc/cmd/Классы/КомандаЗапустить.ossrc/core/Классы/HttpBin.ossrc/internal/Классы/ПодключательКастомныхКонтроллеров.ossrc/internal/Классы/ПровайдерПараметровCFG.ostests/HttpBin_API_test.ostests/oscript.cfg
🧰 Additional context used
🪛 GitHub Actions: Контроль качества
tests/HttpBin_API_test.os
[error] 1-1: Test 'Должен_ПроверитьТочкуМаршрута_Robots' failed. WebException: Connection refused to http://[::]:3333 (3333).
[error] 1-1: Test 'Должен_ПроверитьТочкуМаршрута_IP' failed. WebException: Connection refused to http://127.0.0.1:3333 (3333).
[error] 1-1: Test 'Должен_ПроверитьТочкуМаршрута_Uuid5' failed. WebException: Connection refused to http://127.0.0.1:3333 (3333).
[error] 1-1: Test 'Должен_ПроверитьТочкуМаршрута_Headers' failed. WebException: Connection refused to http://127.0.0.1:3333 (3333).
[error] 1-1: Test 'Должен_ПроверитьТочкуМаршрута_Get' failed. WebException: Connection refused to http://127.0.0.1:3333 (3333).
[error] 1-1: Test 'Должен_ПроверитьТочкуМаршрута_Anything_GET' failed. WebException: Connection refused to http://127.0.0.1:3333 (3333).
[error] 1-1: Test 'Должен_ПроверитьТочкуМаршрута_Anything_PUT' failed. WebException: Connection refused to http://127.0.0.1:3333 (3333).
[error] 1-1: Test 'Должен_ПроверитьТочкуМаршрута_Post_ТелоСтрока' failed. WebException: Connection refused to http://127.0.0.1:3333 (3333).
[error] 1-1: Test 'Должен_ПроверитьТочкуМаршрута_Post_ТелоДвоичныеДанные' failed. WebException: Connection refused to http://127.0.0.1:3333 (3333).
[error] 1-1: Test 'Должен_ПроверитьТочкуМаршрута_Post_ТелоСтрокаJson' failed. WebException: Connection refused to http://127.0.0.1:3333 (3333).
🔇 Additional comments (11)
.github/workflows/qa.yml (1)
10-14: Проверьте, чтоsonar.yml@mainпринимаетoscript_version: stable(и это ожидаемое значение).Это внешний reusable-workflow; если он ждёт другие значения (
default/ltsи т.п.), пайплайн упадёт на валидации/рантайме.src/internal/Классы/ПодключательКастомныхКонтроллеров.os (1)
52-60: Проверьте сигнатуруНайтиФайлы(..., ..., Истина)и подумайте о защите от коллизий/лишних файлов при рекурсии.Если 3-й параметр включает подкаталоги, то:
- возрастает риск подключить “не те”
.os(например, из вложенных служебных папок),- возрастает риск коллизий по
ИмяБезРасширения(одинаковые имена в разных каталогах).src/cmd/Классы/КомандаЗапустить.os (1)
28-36:parent-pidскрыт в справке — ок, но проверьте, что он не нужен пользователям CLI (отладка/оркестрация).Это UX/операционное решение: скрытие усложняет диагностику при запуске вне “родительского” раннера.
tests/oscript.cfg (1)
1-1: Проверьте, относительно чего резолвится../oscript_modulesв CI/тест-раннере (CWD vs папка cfg).Если резолвится от корня репо,
../oscript_modulesуедет “наружу” и загрузка модулей сломается.src/internal/Классы/ПровайдерПараметровCFG.os (1)
1-76: The concern about_Логbeing uninitialized is based on a flawed assumption about OneScript's object lifecycle. When a class is instantiated with theНовыйkeyword (asПровайдерПараметровCFGis inHttpBin.os), the runtime automatically invokesПриСозданииОбъекта()before any public methods can be called. This same initialization pattern is used throughout the codebase in other classes likeКонтроллерРодительскогоПроцесса.osand works correctly. The proposed lazy initialization fix is unnecessary.Likely an incorrect or invalid review comment.
src/core/Классы/HttpBin.os (4)
7-8: New imports support the configuration-based library loading feature.The
configorimport providesМенеджерПараметровfor configuration management, and the internal module providesПровайдерПараметровCFGfor CFG file parsing.
280-280: Environment propagation updated for configuration support.The change to use
ПолучитьПараметрыСредыПроцесса()instead ofПеременныеСреды()enables custom library paths from the config file to be passed to the child process.
448-460: Configuration serialization looks correct.The function properly converts the configuration map back to a semicolon-delimited string, lowercasing keys for consistency.
462-482: Path resolution procedure handles edge cases well.The procedure properly handles:
- Empty catalog values (early return)
- Semicolon-separated paths (takes first value)
- Path normalization with existence check
Note: The
СтартовыйСценарий()usage here shares the same concern flagged above regarding path resolution in test contexts.tests/HttpBin_API_test.os (1)
17-26: The HttpBin service fails to start because environment configuration is not being passed to the child process.The
ПолучитьПараметрыСредыПроцессаfunction (line 398-425) attempts to locateoscript.cfgand read library paths from it, but the path resolution is incorrect:
- Line 401 uses
СтартовыйСценарий().Каталог, which returns the test runner's starting directory (repository root)- The function then looks for
oscript.cfgat./oscript.cfg(line 402)- However,
oscript.cfgexists attests/oscript.cfg, not at the repository root- When the file is not found (line 404), the function silently returns without setting the
OSCRIPT_CONFIGenvironment variable- The child HttpBin process inherits incomplete environment variables and fails to start or load its dependencies
- This explains the "Connection refused to http://127.0.0.1:3333" errors in the pipeline
The issue is in lines 401-405. The function needs to either search in the
tests/subdirectory or use a more robust approach to locate the configuration file. Additionally, wrapping theЗапустить()call with error handling will help surface startup failures instead of silently timing out.packagedef (1)
16-16: Dependency updates are appropriate and actively used in the codebase.The
winowversion bump to 0.11.2 and newconfigordependency at 0.11.0 are both used throughout the codebase.winowis a web framework (referenced in README as WINOW) andconfigoris imported in the HttpBin class for the CFG-based parameter provider. These changes align with the PR's configuration-based library loading enhancements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @tests/HttpBin_API_test.os:
- Around line 29-33: В процедуре ПослеЗапускаТестов защитить вызов остановки
тестового сервера: перед вызовом HttpBin.Остановить() проверить, что HttpBin не
равен Неопределено (и при желании — что у него есть метод Остановить), и
обернуть вызов в блок обработки исключений (поймать и
проигнорировать/залогировать ошибку), чтобы не позволять ошибке при остановке
скрыть исходную проблему запуска.
- Around line 16-25: The test setup sets the HTTPBIN_TESTING_MODE env var in the
ПередЗапускомТестов procedure but never clears it; add an &ПослеВсех teardown
that calls ОчиститьПеременнуюСреды("HTTPBIN_TESTING_MODE") and stops the HttpBin
instance (e.g., HttpBin. Остановить() or similar) if defined; locate the test
file procedures ПередЗапускомТестов and add an &ПослеВсех block that checks if
HttpBin <> Неопределено then clears the env var via
ОчиститьПеременнуюСреды("HTTPBIN_TESTING_MODE") and cleans up the HttpBin
resource to avoid leaking the 400ms socket delay into other suites.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/core/Классы/HttpBin.ostests/HttpBin_API_test.os
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: test (lts, 1testrunner) / build (windows-latest, lts, 1testrunner)
- GitHub Check: test (dev, oneunit) / build (ubuntu-latest, dev, oneunit)
- GitHub Check: test (stable, oneunit) / build (ubuntu-latest, stable, oneunit)
- GitHub Check: test (dev, oneunit) / build (windows-latest, dev, oneunit)
- GitHub Check: test (stable, oneunit) / build (windows-latest, stable, oneunit)
- GitHub Check: test (default, 1testrunner) / build (windows-latest, default, 1testrunner)
- GitHub Check: test (default, 1testrunner) / build (macos-latest, default, 1testrunner)
- GitHub Check: sonar / test
- GitHub Check: test (stable, oneunit) / build (ubuntu-latest, stable, oneunit)
- GitHub Check: test (dev, oneunit) / build (ubuntu-latest, dev, oneunit)
- GitHub Check: test (lts, 1testrunner) / build (macos-latest, lts, 1testrunner)
- GitHub Check: test (default, 1testrunner) / build (macos-latest, default, 1testrunner)
- GitHub Check: test (stable, oneunit) / build (windows-latest, stable, oneunit)
- GitHub Check: test (dev, oneunit) / build (windows-latest, dev, oneunit)
- GitHub Check: test (lts, 1testrunner) / build (ubuntu-latest, lts, 1testrunner)
- GitHub Check: test (stable, oneunit) / build (macos-latest, stable, oneunit)
- GitHub Check: test (lts, 1testrunner) / build (windows-latest, lts, 1testrunner)
- GitHub Check: test (default, 1testrunner) / build (ubuntu-latest, default, 1testrunner)
- GitHub Check: test (default, 1testrunner) / build (windows-latest, default, 1testrunner)
🔇 Additional comments (2)
src/core/Классы/HttpBin.os (2)
7-9: configor is actively used and necessary — no action required.The
configorlibrary (version 0.10.0, pinned) is not unused. It provides theМенеджерПараметровclass used in theПолучитьПараметрыСредыПроцесса()function (lines 413–419) to read theoscript.cfgconfiguration file and load library paths. The API methods used (УстановитьФайлПараметров,ДобавитьПровайдерПараметров,Прочитать,Параметр) are standard and stable for a configuration manager.
280-281: No action needed — env parameter is in the correct position.The
СоздатьПроцессfunction signature positions environment variables as the 6th parameter (after cmdLine, currentDir, redirectOutput, redirectInput, and encoding). Your code correctly passesПолучитьПараметрыСредыПроцесса()at this position.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ПолучитьПараметрыСредыПроцесса: есть несколько edge-case’ов, которые могут ломать старт/конфиг.
- Не мутировать “живые” переменные среды родителя (если API возвращает view, а не копию). Сейчас вы меняете
ПеременныеСреды["OSCRIPT_CONFIG"]напрямую. Лучше собрать копию и менять её. - Ранний
Возврат, если заполненlib.system, блокирует установкуlib.additional. Если это не намеренно — лучше “пропускать только lib.system”, но не выходить из функции. - Нет try/catch вокруг чтения конфигурации. Любая проблема с
oscript.cfg/провайдером/парсингом может валить запуск сервиса целиком — часто лучше залогировать и продолжить со старой средой. КонфигурацияОСкрпитВСоответствие: стоит триммить и значения тоже (пути из cfg часто с пробелами).УстановитьКаталогВКонфигурацию: поведение с абсолютными путями зависит отОбъединитьПути. ЕслиКаталогможет быть абсолютным — лучше не “прибивать” его кСтартовыйСценарий().Каталог(используйте проверку абсолютности, если она есть вfs).
Proposed fix (минимально инвазивный)
Функция ПолучитьПараметрыСредыПроцесса()
- ПеременныеСреды = ПеременныеСреды();
+ ПеременныеСредыИсходные = ПеременныеСреды();
+ ПеременныеСреды = Новый Соответствие();
+ Для Каждого КлючИЗначение Из ПеременныеСредыИсходные Цикл
+ ПеременныеСреды.Вставить(КлючИЗначение.Ключ, КлючИЗначение.Значение);
+ КонецЦикла;
СтартовыйКаталог = СтартовыйСценарий().Каталог;
ПутьККонфигурации = ОбъединитьПути(СтартовыйКаталог, "oscript.cfg");
Если Не ФС.ФайлСуществует(ПутьККонфигурации) Тогда
Возврат ПеременныеСреды;
КонецЕсли;
Конфигурация = КонфигурацияОСкрпитВСоответствие(ПеременныеСреды["OSCRIPT_CONFIG"]);
- Если ЗначениеЗаполнено(Конфигурация["lib.system"]) Тогда
- Возврат ПеременныеСреды;
- КонецЕсли;
+ // Не выходим сразу: lib.additional может быть полезен даже если lib.system задан извне.
- МенеджерПараметров = Новый МенеджерПараметров();
- МенеджерПараметров.УстановитьФайлПараметров(ПутьККонфигурации);
- МенеджерПараметров.ДобавитьПровайдерПараметров(Новый ПровайдерПараметровCFG());
- МенеджерПараметров.Прочитать();
+ Попытка
+ МенеджерПараметров = Новый МенеджерПараметров();
+ МенеджерПараметров.УстановитьФайлПараметров(ПутьККонфигурации);
+ МенеджерПараметров.ДобавитьПровайдерПараметров(Новый ПровайдерПараметровCFG());
+ МенеджерПараметров.Прочитать();
+ Исключение
+ _Лог.Отладка(
+ "Не удалось прочитать oscript.cfg (%1). Ошибка: %2",
+ ПутьККонфигурации,
+ ПодробноеПредставлениеОшибки(ИнформацияОбОшибке())
+ );
+ Возврат ПеременныеСреды;
+ КонецПопытки;
- УстановитьКаталогВКонфигурацию(Конфигурация, "lib.system", МенеджерПараметров.Параметр("lib.system"));
+ Если Не ЗначениеЗаполнено(Конфигурация["lib.system"]) Тогда
+ УстановитьКаталогВКонфигурацию(Конфигурация, "lib.system", МенеджерПараметров.Параметр("lib.system"));
+ КонецЕсли;
УстановитьКаталогВКонфигурацию(Конфигурация, "lib.additional", МенеджерПараметров.Параметр("lib.additional"));
ПеременныеСреды["OSCRIPT_CONFIG"] = КонфигурацияОСкрпитВСтроку(Конфигурация);
Возврат ПеременныеСреды;
КонецФункции
Функция КонфигурацияОСкрпитВСоответствие(Параметры)
Результат = Новый Соответствие();
Если Не ЗначениеЗаполнено(Параметры) Тогда
Возврат Результат;
КонецЕсли;
КлючиИЗначения = СтрРазделить(Параметры, ";");
Для Каждого КлючИЗначение Из КлючиИЗначения Цикл
ПозицияРавенства = СтрНайти(КлючИЗначение, "=");
Если ПозицияРавенства > 0 Тогда
ЛеваяЧасть = Сред(КлючИЗначение, 1, ПозицияРавенства - 1);
ПраваяЧасть = Сред(КлючИЗначение, ПозицияРавенства + 1);
- Результат.Вставить(СокрЛП(ЛеваяЧасть), ПраваяЧасть);
+ Результат.Вставить(СокрЛП(ЛеваяЧасть), СокрЛП(ПраваяЧасть));
КонецЕсли;
КонецЦикла;
Возврат Результат;
КонецФункции📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Функция ПолучитьПараметрыСредыПроцесса() | |
| ПеременныеСреды = ПеременныеСреды(); | |
| СтартовыйКаталог = СтартовыйСценарий().Каталог; | |
| ПутьККонфигурации = ОбъединитьПути(СтартовыйКаталог, "oscript.cfg"); | |
| Если Не ФС.ФайлСуществует(ПутьККонфигурации) Тогда | |
| Возврат ПеременныеСреды; | |
| КонецЕсли; | |
| Конфигурация = КонфигурацияОСкрпитВСоответствие(ПеременныеСреды["OSCRIPT_CONFIG"]); | |
| Если ЗначениеЗаполнено(Конфигурация["lib.system"]) Тогда | |
| Возврат ПеременныеСреды; | |
| КонецЕсли; | |
| МенеджерПараметров = Новый МенеджерПараметров(); | |
| МенеджерПараметров.УстановитьФайлПараметров(ПутьККонфигурации); | |
| МенеджерПараметров.ДобавитьПровайдерПараметров(Новый ПровайдерПараметровCFG()); | |
| МенеджерПараметров.Прочитать(); | |
| УстановитьКаталогВКонфигурацию(Конфигурация, "lib.system", МенеджерПараметров.Параметр("lib.system")); | |
| УстановитьКаталогВКонфигурацию(Конфигурация, "lib.additional", МенеджерПараметров.Параметр("lib.additional")); | |
| ПеременныеСреды["OSCRIPT_CONFIG"] = КонфигурацияОСкрпитВСтроку(Конфигурация); | |
| Возврат ПеременныеСреды; | |
| КонецФункции | |
| Функция КонфигурацияОСкрпитВСоответствие(Параметры) | |
| Результат = Новый Соответствие(); | |
| Если Не ЗначениеЗаполнено(Параметры) Тогда | |
| Возврат Результат; | |
| КонецЕсли; | |
| КлючиИЗначения = СтрРазделить(Параметры, ";"); | |
| Для Каждого КлючИЗначение Из КлючиИЗначения Цикл | |
| ПозицияРавенства = СтрНайти(КлючИЗначение, "="); | |
| Если ПозицияРавенства > 0 Тогда | |
| ЛеваяЧасть = Сред(КлючИЗначение, 1, ПозицияРавенства - 1); | |
| ПраваяЧасть = Сред(КлючИЗначение, ПозицияРавенства + 1); | |
| Результат.Вставить(СокрЛП(ЛеваяЧасть), ПраваяЧасть); | |
| КонецЕсли; | |
| КонецЦикла; | |
| Возврат Результат; | |
| КонецФункции | |
| Функция КонфигурацияОСкрпитВСтроку(Параметры) | |
| Результат = Новый Массив(); | |
| Для Каждого КлючИЗначение Из Параметры Цикл | |
| Если ЗначениеЗаполнено(КлючИЗначение.Значение) Тогда | |
| Результат.Добавить(СтрШаблон("%1=%2", НРег(КлючИЗначение.Ключ), КлючИЗначение.Значение)); | |
| КонецЕсли; | |
| КонецЦикла; | |
| Возврат СтрСоединить(Результат, ";"); | |
| КонецФункции | |
| Процедура УстановитьКаталогВКонфигурацию(Конфигурация, Ключ, Знач Каталог) | |
| Если Не ЗначениеЗаполнено(Каталог) Тогда | |
| Возврат; | |
| КонецЕсли; | |
| ПозицияТочкиЗапятой = СтрНайти(Каталог, ";"); | |
| Если ПозицияТочкиЗапятой > 0 Тогда | |
| Каталог = Сред(Каталог, 1, ПозицияТочкиЗапятой - 1); | |
| КонецЕсли; | |
| СтартовыйКаталог = СтартовыйСценарий().Каталог; | |
| НормализованныйПуть = ФС.НормализоватьПуть(ОбъединитьПути(СтартовыйКаталог, Каталог)); | |
| Если ФС.КаталогСуществует(НормализованныйПуть) Тогда | |
| Каталог = НормализованныйПуть; | |
| КонецЕсли; | |
| Конфигурация[Ключ] = Каталог; | |
| КонецПроцедуры | |
| Функция ПолучитьПараметрыСредыПроцесса() | |
| ПеременныеСредыИсходные = ПеременныеСреды(); | |
| ПеременныеСреды = Новый Соответствие(); | |
| Для Каждого КлючИЗначение Из ПеременныеСредыИсходные Цикл | |
| ПеременныеСреды.Вставить(КлючИЗначение.Ключ, КлючИЗначение.Значение); | |
| КонецЦикла; | |
| СтартовыйКаталог = СтартовыйСценарий().Каталог; | |
| ПутьККонфигурации = ОбъединитьПути(СтартовыйКаталог, "oscript.cfg"); | |
| Если Не ФС.ФайлСуществует(ПутьККонфигурации) Тогда | |
| Возврат ПеременныеСреды; | |
| КонецЕсли; | |
| Конфигурация = КонфигурацияОСкрпитВСоответствие(ПеременныеСреды["OSCRIPT_CONFIG"]); | |
| // Не выходим сразу: lib.additional может быть полезен даже если lib.system задан извне. | |
| Попытка | |
| МенеджерПараметров = Новый МенеджерПараметров(); | |
| МенеджерПараметров.УстановитьФайлПараметров(ПутьККонфигурации); | |
| МенеджерПараметров.ДобавитьПровайдерПараметров(Новый ПровайдерПараметровCFG()); | |
| МенеджерПараметров.Прочитать(); | |
| Исключение | |
| _Лог.Отладка( | |
| "Не удалось прочитать oscript.cfg (%1). Ошибка: %2", | |
| ПутьККонфигурации, | |
| ПодробноеПредставлениеОшибки(ИнформацияОбОшибке()) | |
| ); | |
| Возврат ПеременныеСреды; | |
| КонецПопытки; | |
| Если Не ЗначениеЗаполнено(Конфигурация["lib.system"]) Тогда | |
| УстановитьКаталогВКонфигурацию(Конфигурация, "lib.system", МенеджерПараметров.Параметр("lib.system")); | |
| КонецЕсли; | |
| УстановитьКаталогВКонфигурацию(Конфигурация, "lib.additional", МенеджерПараметров.Параметр("lib.additional")); | |
| ПеременныеСреды["OSCRIPT_CONFIG"] = КонфигурацияОСкрпитВСтроку(Конфигурация); | |
| Возврат ПеременныеСреды; | |
| КонецФункции | |
| Функция КонфигурацияОСкрпитВСоответствие(Параметры) | |
| Результат = Новый Соответствие(); | |
| Если Не ЗначениеЗаполнено(Параметры) Тогда | |
| Возврат Результат; | |
| КонецЕсли; | |
| КлючиИЗначения = СтрРазделить(Параметры, ";"); | |
| Для Каждого КлючИЗначение Из КлючиИЗначения Цикл | |
| ПозицияРавенства = СтрНайти(КлючИЗначение, "="); | |
| Если ПозицияРавенства > 0 Тогда | |
| ЛеваяЧасть = Сред(КлючИЗначение, 1, ПозицияРавенства - 1); | |
| ПраваяЧасть = Сред(КлючИЗначение, ПозицияРавенства + 1); | |
| Результат.Вставить(СокрЛП(ЛеваяЧасть), СокрЛП(ПраваяЧасть)); | |
| КонецЕсли; | |
| КонецЦикла; | |
| Возврат Результат; | |
| КонецФункции | |
| Процедура УстановитьКаталогВКонфигурацию(Конфигурация, Ключ, Знач Каталог) | |
| Если Не ЗначениеЗаполнено(Каталог) Тогда | |
| Возврат; | |
| КонецЕсли; | |
| ПозицияТочкиЗапятой = СтрНайти(Каталог, ";"); | |
| Если ПозицияТочкиЗапятой > 0 Тогда | |
| Каталог = Сред(Каталог, 1, ПозицияТочкиЗапятой - 1); | |
| КонецЕсли; | |
| СтартовыйКаталог = СтартовыйСценарий().Каталог; | |
| НормализованныйПуть = ФС.НормализоватьПуть(ОбъединитьПути(СтартовыйКаталог, Каталог)); | |
| Если ФС.КаталогСуществует(НормализованныйПуть) Тогда | |
| Каталог = НормализованныйПуть; | |
| КонецЕсли; | |
| Конфигурация[Ключ] = Каталог; | |
| КонецПроцедуры |
| &ПередВсеми | ||
| Процедура ПередЗапускомТестов() Экспорт | ||
|
|
||
| СервисЗапущен = Ложь; | ||
|
|
||
| УстановитьПеременнуюСреды("HTTPBIN_IS_TEST_MODE", "true"); | ||
|
|
||
| HttpBin = Новый HttpBin(); | ||
| HttpBin.Запустить(); | ||
|
|
||
| СервисЗапущен = Истина; | ||
|
|
||
| Если HttpBin = Неопределено Тогда | ||
| УстановитьПеременнуюСреды("HTTPBIN_TESTING_MODE", "true"); | ||
|
|
||
| HttpBin = Новый HttpBin(); | ||
| HttpBin.УстановитьТаймаутЗапуска(10); | ||
| HttpBin.Запустить(); | ||
| КонецЕсли; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
rg -n "HTTPBIN_TESTING_MODE|HTTPBIN_IS_TEST_MODE" -SRepository: Stivo182/oscript-httpbin
Length of output: 266
🏁 Script executed:
# Check for teardown procedures and environment variable handling
rg -n "ПослеВсеми|ОчиститьПеременнуюСреды|УстановитьПеременнуюСреды" tests/ -A 2Repository: Stivo182/oscript-httpbin
Length of output: 247
🏁 Script executed:
# List test files to understand the test structure
find tests/ -type f -name "*.os" | head -20Repository: Stivo182/oscript-httpbin
Length of output: 174
🏁 Script executed:
# Check the ЗапускательСервиса.os file to see how HTTPBIN_TESTING_MODE is used
wc -l src/cmd/Классы/ЗапускательСервиса.osRepository: Stivo182/oscript-httpbin
Length of output: 107
🏁 Script executed:
# Read the context around line 38 where HTTPBIN_TESTING_MODE is used
sed -n '30,50p' src/cmd/Классы/ЗапускательСервиса.osRepository: Stivo182/oscript-httpbin
Length of output: 386
🏁 Script executed:
# Read the entire HttpBin_API_test.os file to check for teardown procedures
wc -l tests/HttpBin_API_test.osRepository: Stivo182/oscript-httpbin
Length of output: 98
🏁 Script executed:
# Check the full content of HttpBin_API_test.os
cat -n tests/HttpBin_API_test.osRepository: Stivo182/oscript-httpbin
Length of output: 47030
Add environment variable cleanup in teardown procedure.
The HTTPBIN_TESTING_MODE environment variable is set in &ПередВсеми but never reset. The &ПослеВсех procedure should also call ОчиститьПеременнуюСреды("HTTPBIN_TESTING_MODE") to prevent the variable from persisting if other test suites run in the same process, which would cause those tests to unexpectedly receive the 400ms socket delay configured in ЗапускательСервиса.os:38.
🤖 Prompt for AI Agents
In @tests/HttpBin_API_test.os around lines 16 - 25, The test setup sets the
HTTPBIN_TESTING_MODE env var in the ПередЗапускомТестов procedure but never
clears it; add an &ПослеВсех teardown that calls
ОчиститьПеременнуюСреды("HTTPBIN_TESTING_MODE") and stops the HttpBin instance
(e.g., HttpBin. Остановить() or similar) if defined; locate the test file
procedures ПередЗапускомТестов and add an &ПослеВсех block that checks if
HttpBin <> Неопределено then clears the env var via
ОчиститьПеременнуюСреды("HTTPBIN_TESTING_MODE") and cleans up the HttpBin
resource to avoid leaking the 400ms socket delay into other suites.
| &ПослеВсех | ||
| Процедура ПослеЗапускаТестов() Экспорт | ||
| HttpBin.Остановить(); | ||
| КонецПроцедуры | ||
|
|
||
| &ПередКаждым | ||
| Процедура ПередЗапускомТеста() Экспорт | ||
| Если Не СервисЗапущен Тогда | ||
| ВызватьИсключение "Сервис не запущен"; | ||
| КонецЕсли; | ||
| HttpBin = Неопределено; | ||
| КонецПроцедуры |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ПослеЗапускаТестов: защитить остановку, чтобы teardown не падал и не скрывал причину фейла старта.
Если HttpBin не был создан (или создание/старт упали), то HttpBin.Остановить() в &ПослеВсех сам выбросит исключение и “перекроет” первопричину.
Proposed fix
&ПослеВсех
Процедура ПослеЗапускаТестов() Экспорт
- HttpBin.Остановить();
- HttpBin = Неопределено;
+ Если HttpBin <> Неопределено Тогда
+ HttpBin.Остановить();
+ HttpBin = Неопределено;
+ КонецЕсли;
КонецПроцедуры📝 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.
| &ПослеВсех | |
| Процедура ПослеЗапускаТестов() Экспорт | |
| HttpBin.Остановить(); | |
| КонецПроцедуры | |
| &ПередКаждым | |
| Процедура ПередЗапускомТеста() Экспорт | |
| Если Не СервисЗапущен Тогда | |
| ВызватьИсключение "Сервис не запущен"; | |
| КонецЕсли; | |
| HttpBin = Неопределено; | |
| КонецПроцедуры | |
| &ПослеВсех | |
| Процедура ПослеЗапускаТестов() Экспорт | |
| Если HttpBin <> Неопределено Тогда | |
| HttpBin.Остановить(); | |
| HttpBin = Неопределено; | |
| КонецЕсли; | |
| КонецПроцедуры |
🤖 Prompt for AI Agents
In @tests/HttpBin_API_test.os around lines 29 - 33, В процедуре
ПослеЗапускаТестов защитить вызов остановки тестового сервера: перед вызовом
HttpBin.Остановить() проверить, что HttpBin не равен Неопределено (и при желании
— что у него есть метод Остановить), и обернуть вызов в блок обработки
исключений (поймать и проигнорировать/залогировать ошибку), чтобы не позволять
ошибке при остановке скрыть исходную проблему запуска.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/HttpBin_API_test.os (1)
18-18: Remove extraneous whitespace.This empty line appears to be leftover from removed code. Consider removing it for cleaner code style.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/HttpBin_API_test.os
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: test (stable, oneunit) / build (windows-latest, stable, oneunit)
- GitHub Check: test (lts, 1testrunner) / build (ubuntu-latest, lts, 1testrunner)
- GitHub Check: test (dev, oneunit) / build (windows-latest, dev, oneunit)
- GitHub Check: test (default, 1testrunner) / build (ubuntu-latest, default, 1testrunner)
- GitHub Check: test (stable, oneunit) / build (macos-latest, stable, oneunit)
- GitHub Check: test (lts, 1testrunner) / build (windows-latest, lts, 1testrunner)
- GitHub Check: test (default, 1testrunner) / build (windows-latest, default, 1testrunner)
- GitHub Check: test (dev, oneunit) / build (ubuntu-latest, dev, oneunit)
- GitHub Check: test (stable, oneunit) / build (windows-latest, stable, oneunit)
- GitHub Check: test (default, 1testrunner) / build (ubuntu-latest, default, 1testrunner)
- GitHub Check: test (default, 1testrunner) / build (windows-latest, default, 1testrunner)
- GitHub Check: test (lts, 1testrunner) / build (macos-latest, lts, 1testrunner)
- GitHub Check: test (lts, 1testrunner) / build (windows-latest, lts, 1testrunner)
- GitHub Check: test (lts, 1testrunner) / build (ubuntu-latest, lts, 1testrunner)
- GitHub Check: test (default, 1testrunner) / build (macos-latest, default, 1testrunner)
- GitHub Check: test (stable, oneunit) / build (ubuntu-latest, stable, oneunit)
- GitHub Check: test (dev, oneunit) / build (windows-latest, dev, oneunit)
- GitHub Check: test (dev, oneunit) / build (macos-latest, dev, oneunit)
- GitHub Check: test (dev, oneunit) / build (ubuntu-latest, dev, oneunit)
- GitHub Check: sonar / test
🔇 Additional comments (2)
tests/HttpBin_API_test.os (2)
19-25: Good defensive programming for lazy initialization.The undefined check provides robustness even though
&ПередВсемиshould only execute once. The initialization sequence (set env var → create instance → configure timeout → start) is correct.
31-33: LGTM: Proper cleanup with defensive check.The undefined check ensures cleanup only runs when needed, and resetting
HttpBinto undefined prevents potential reuse issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/HttpBin_test.os (2)
21-31: Сделать очистку_HttpBinустойчивой к исключениям при остановке.Сейчас, если
_HttpBin.Остановить()бросит исключение, строкаLine 28не выполнится, и состояние «залипнет» для следующих тестов.Proposed diff
Если Не _HttpBin = Неопределено Тогда - _HttpBin.Остановить(); - _HttpBin = Неопределено; + Попытка + _HttpBin.Остановить(); + Исключение + // намеренно игнорируем ошибки остановки в cleanup, чтобы не ломать следующие тесты + КонецПопытки; + _HttpBin = Неопределено; КонецЕсли;
44-65: Асинх-тест может флапать из-за ожидания “по времени запуска” (Line 60).
Приостановить(ВремяЗапуска)часто нестабилен в CI; лучше ждать “пока отвечает” с таймаутом.
🧹 Nitpick comments (1)
tests/HttpBin_test.os (1)
131-137: Подумать о переименованииHttpBin()хелпера, чтобы избежать путаницы/шейдинга типаHttpBin(Line 135-136).Proposed diff
-Функция HttpBin() - Возврат Новый HttpBin().УстановитьТаймаутЗапуска(10); -КонецФункции +Функция СоздатьHttpBin() + Возврат Новый HttpBin().УстановитьТаймаутЗапуска(10); +КонецФункцииПроверьте, что в OneScript имя функции
HttpBinне конфликтует с именем типаHttpBin(особенно в выраженииНовый HttpBin()внутри этой же функции).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/HttpBin_test.os
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: test (dev, oneunit) / build (ubuntu-latest, dev, oneunit)
- GitHub Check: test (stable, oneunit) / build (ubuntu-latest, stable, oneunit)
- GitHub Check: test (default, 1testrunner) / build (ubuntu-latest, default, 1testrunner)
- GitHub Check: test (stable, oneunit) / build (windows-latest, stable, oneunit)
- GitHub Check: test (lts, 1testrunner) / build (ubuntu-latest, lts, 1testrunner)
- GitHub Check: test (lts, 1testrunner) / build (windows-latest, lts, 1testrunner)
- GitHub Check: test (stable, oneunit) / build (macos-latest, stable, oneunit)
- GitHub Check: test (default, 1testrunner) / build (macos-latest, default, 1testrunner)
- GitHub Check: test (dev, oneunit) / build (windows-latest, dev, oneunit)
- GitHub Check: test (lts, 1testrunner) / build (macos-latest, lts, 1testrunner)
- GitHub Check: test (dev, oneunit) / build (macos-latest, dev, oneunit)
- GitHub Check: test (dev, oneunit) / build (ubuntu-latest, dev, oneunit)
- GitHub Check: test (default, 1testrunner) / build (ubuntu-latest, default, 1testrunner)
- GitHub Check: test (stable, oneunit) / build (windows-latest, stable, oneunit)
- GitHub Check: test (stable, oneunit) / build (macos-latest, stable, oneunit)
- GitHub Check: test (default, 1testrunner) / build (windows-latest, default, 1testrunner)
- GitHub Check: test (lts, 1testrunner) / build (ubuntu-latest, lts, 1testrunner)
- GitHub Check: test (stable, oneunit) / build (ubuntu-latest, stable, oneunit)
- GitHub Check: sonar / test
- GitHub Check: test (lts, 1testrunner) / build (windows-latest, lts, 1testrunner)
🔇 Additional comments (3)
tests/HttpBin_test.os (3)
33-42: Упрощение старта черезHttpBin()выглядит ок (Line 37).
67-115: ИнициализацияHttpBin()+ относительные пути контроллеров выглядят консистентно (Line 71-74, 87-90, 105-108).
117-129: Ок: старт сервиса при измененномТекущийКаталогчерезHttpBin()(Line 124).
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.