-
Notifications
You must be signed in to change notification settings - Fork 5
Исправлена ошибка при регистрации тестовых наборов из файлов с именами не являющимися валидными идентификаторами #16
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
…и не являющимися валидными идентификаторами
ПрохождениеДобавлена переменная с регулярным выражением для обработки имён классов, обновлена логика формирования имён тестовых наборов с заменой префикса с Изменения
Диаграмма последовательностиsequenceDiagram
participant Test as Тестовая система
participant Discovery as Обнаружитель
participant Suite as ОпределениеТестНабора
participant Template as ЗаготовкаТестирование
Test->>Discovery: ПроверитьТестНабор(Набор, Имя, ..., ИмяФайла="")
activate Discovery
alt ИмяФайла пуст
Discovery->>Discovery: ИмяФайла = Имя
end
Discovery->>Discovery: Путь = Путь_к_файлу + ".os"
Discovery->>Suite: Создание объекта тестовой системы
activate Suite
Suite->>Template: Получить РегулярноеВыражениеИмяКласса
activate Template
Template->>Template: Выполнить замену oneunit_testsuite_ на ""
Template-->>Suite: Обновлённое имя (_Имя)
deactivate Template
Suite-->>Discovery: Объект тестового набора готов
deactivate Suite
Discovery-->>Test: Проверка завершена
deactivate Discovery
Оценка усилий рецензирования🎯 3 (Умеренная) | ⏱️ ~25 минут
Стихотворение
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
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: 1
🧹 Nitpick comments (3)
src/core/internal/Классы/ЗаготовкаТестирование.os (1)
8-8: Санитизация имени файла черезРегулярноеВыражениеИмяКлассавыглядит корректнойЛогика формирования имени набора
"oneunit_testsuite_" + РегулярноеВыражениеИмяКласса.Заменить(Тест.ИмяБезРасширения, "_")
хорошо решает задачу: все символы, не подходящие под «идентификатор» (включая первый недопустимый символ), заменяются на_, при этом буквы (латиница/кириллица), цифры и_сохраняются. Это даёт ожидаемое имя вида__Тест_Набор_Со_Странным_Именем_1_для нового тестового файла и согласуется с проверками вtests/Обнаружитель.os.В качестве небольшого улучшения можно:
- Добавить короткий комментарий рядом с инициализацией
РегулярноеВыражениеИмяКласса, поясняющий, что первый альтернативный блок паттерна отвечает за первый символ, а второй — за все последующие.- (Опционально) Сделать переменную локальной в
ПриИнициализацииПоделкиилиПриСозданииОбъекта, если нет планов переиспользования её в других процедурах модуля.Also applies to: 27-29, 109-110
tests/Обнаружитель.os (2)
19-19: Обновлённое ожидаемое количество наборов (15) соответствует добавленному тестовому файлуУвеличение ожидаемого количества дочерних элементов плана тестов до
15логично с учётом нового набора__Тест_Набор_Со_Странным_Именем_1_. Важно не забывать поддерживать это значение в актуальном состоянии при дальнейшем добавлении/удалении фикстур.
1163-1171: Новый параметрИмяФайлаи проверка типа тест-набора корректно покрывают кейс «имя файла ≠ имя набора»Изменения в
ПроверитьТестНабор:
- расширение сигнатуры до
...(ТестНабор, Имя, Теги, Таймаут, Условия, ЖизненныйЦикл, ЭтоОдиночка = Ложь, ПроверятьТестНабор = Истина, ИмяФайла = "")- установка
ИмяФайла = Имя, если он не задан явно;- вычисление
ПутьчерезИмяФайла + ".os";- проверка типа
ТестНабор.ТестНабор()на"oneunit_testsuite_" + Имядают:
- полную обратную совместимость для старых тестов (когда
ИмяФайлане передаётся — путь остаётся прежним),- возможность явно указать имя файла, отличное от логического имени набора (что и нужно для невалидных идентификаторов),
- согласованность с новой схемой имён классов (
oneunit_testsuite_+ санитизированное имя).Сама реализация выглядит чистой и читабельной, без лишних побочных эффектов.
Also applies to: 1180-1181
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/core/internal/Классы/ЗаготовкаТестирование.os(3 hunks)src/core/internal/Классы/ОпределениеТестНабора.os(1 hunks)tests/fixtures/1_Тест-Набор.Со+Странным^Именем_1,.os(1 hunks)tests/Обнаружитель.os(3 hunks)
⏰ 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). (2)
- GitHub Check: test (dev) / build (ubuntu-latest, dev)
- GitHub Check: test (default) / build (windows-latest, default)
🔇 Additional comments (2)
tests/fixtures/1_Тест-Набор.Со+Странным^Именем_1,.os (1)
1-8: Фикстура выглядит корректной и минимальнойСтруктура файла (экспортируемый тест и пустой конструктор набора с &ТестовыйНабор/&Порядок) соответствует ожидаемому шаблону и хорошо подходит для проверки обнаружения наборов с «странными» именами файлов.
src/core/internal/Классы/ОпределениеТестНабора.os (1)
122-123: Префиксoneunit_testsuite_согласован с новой логикой генерации имёнЗамена префикса на
"oneunit_testsuite_"при вычислении_Имявыравнивает представление тестового набора с тем, как оно теперь формируется вЗаготовкаТестирование.os. Это делаетИмя()и строковое представление объекта согласованными с проверкой типа вПроверитьТестНабор("oneunit_testsuite_" + Имя).С учётом добавленных тестов в
tests/Обнаружитель.osизменение выглядит корректным.
tests/Обнаружитель.os
Outdated
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.
Вызов ПроверитьТестНабор с голыми , , выглядит как ошибка и может не работать как ожидается
В новом блоке для набора __Тест_Набор_Со_Странным_Именем_1_ вызов:
ПроверитьТестНабор(
...,
Карты.ИзЭлементов(
"ПередВсеми", Списки.ИзЭлементов(),
"ПередКаждым", Списки.ИзЭлементов(),
"ПослеВсех", Списки.ИзЭлементов(),
"ПослеКаждого", Списки.ИзЭлементов()
),
,
,
"1_Тест-Набор.Со+Странным^Именем_1,"
);содержит два пустых аргумента (, ,), что выглядит как опечатка/незавершённое редактирование и делает намерение вызова неочевидным. Судя по сигнатуре процедуры, цель — использовать значения по умолчанию для параметров ЭтоОдиночка и ПроверятьТестНабор, но при этом передать ненулевой ИмяФайла.
Рекомендуемый вариант — явно передать нужный последний параметр, например:
ПроверитьТестНабор(
...,
Карты.ИзЭлементов(
"ПередВсеми", Списки.ИзЭлементов(),
"ПередКаждым", Списки.ИзЭлементов(),
"ПослеВсех", Списки.ИзЭлементов(),
"ПослеКаждого", Списки.ИзЭлементов()
),
ИмяФайла = "1_Тест-Набор.Со+Странным^Именем_1,"
);или, если принципиально позиционный вызов, – явно указать флаги:
...,
Карты.ИзЭлементов(...),
Ложь,
Истина,
"1_Тест-Набор.Со+Странным^Именем_1,"
);Так вызов станет однозначным и не будет зависеть от трактовки «пустых» параметров.
🤖 Prompt for AI Agents
tests/Обнаружитель.os lines 1121-1157: The call to ПроверитьТестНабор includes
two empty positional arguments (`, ,`) which is ambiguous and likely a typo;
replace them with explicit arguments so the intent is clear — either supply the
missing boolean flags positionally (e.g. Ложь, Истина) followed by the filename,
or use a named parameter for the filename (e.g. ИмяФайла =
"1_Тест-Набор.Со+Странным^Именем_1,") so the call is unambiguous and
compiles/behaves as intended.
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.
Кролик, OneScript не поддерживает именованные параметры
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.
@sfaqer, спасибо за разъяснение! Понял, что OneScript не поддерживает именованные параметры.
В этом случае использование пустых позиционных аргументов (, ,) для пропуска необязательных параметров ЭтоОдиночка и ПроверятьТестНабор (чтобы они приняли значения по умолчанию) и передачи последнего параметра ИмяФайла является корректным подходом для OneScript. Приношу извинения за путаницу в моём первоначальном комментарии.
Код написан правильно для данной платформы! 🐰
/)/)
(^.^)
(")_(") благодарю за уточнение!
✏️ Learnings added
Learnt from: sfaqer
Repo: sfaqer/OneUnit PR: 16
File: tests/Обнаружитель.os:1121-1157
Timestamp: 2025-11-21T04:58:16.916Z
Learning: OneScript does not support named parameters. To skip optional parameters while providing later ones, use empty positional arguments (e.g., `Функция(Required1, Required2, , , OptionalLast)`) to apply default values.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
closes: #11
Summary by CodeRabbit
Примечания к выпуску
Новые функции
Чинки
✏️ Tip: You can customize this high-level summary in your review settings.