Переработка блока работы с состояниями, предположения, фиксы, рефакторинг #15
Conversation
…отдельный жизненный цикл, а часть общего цикла результата выполнения теста с состоянием "Пропущен" * Тестовые наборы выкидывающие исключение из конструктора теперь корректно помечаются как сломанные * Тесты сломанных наборов больше не репортятся * Добавлена обработка исключений типа ошибка утверждения из asserts, если тест падает с такой ошибкой, он помечается как "Ошибка" если падает с другим исключением то как "Сломан" * Добавлены "Предположения" которые позволяют прервать тест в процессе выполнения * Рефакторинг
WalkthroughДобавлены механизмы предположений для прерывания тестов; внутренняя модель результата теста переименована в "Состояние" и распространена по исполнителю, репортерам и статистике; репортеры и логика вывода переписаны на использование единой статистики; добавлены тестовые фикстуры и документация для модуля Предполагаем. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Тест
participant Executor as АсинхронныйИсполнитель
participant Assumptions as Предполагаем
participant AbortEx as ИсключениеТестПрерван
participant Stats as РепортерСтатистика
Test->>Executor: ВыполнитьТест(Тест)
Executor->>Executor: ПроверитьСозданиеНабора / ПропуститьОпределение?
Executor->>Test: Запустить тело теста
Test->>Assumptions: ЧтоИстинно(условие, сообщение)
alt условие = Истина
Assumptions-->>Test: продолжить
else условие = Ложь
Assumptions->>AbortEx: ПрерватьТест -> выброс ИсключениеТестПрерван
AbortEx-->>Executor: перехват прерывания
Executor->>Stats: Записать Результат(Тест).Состояние = Прерван
end
Executor->>Executor: ОбъединитьРезультатыВыполнения(набор, тест)
Executor->>Stats: Сохранить Результат(Тест)
sequenceDiagram
participant Reporter as Репортер
participant Stats as РепортерСтатистика
participant Results as Результаты
participant States as СостоянияВыполненияТестов
Reporter->>Stats: Результат(Определение)?
Stats->>Results: Получить(Определение)
Results-->>Stats: {Состояние, Причины, Вывод, Время}
Stats->>States: НаихудшееСостояние(a, b)
States-->>Stats: состояние_с_макс_приоритетом
Stats-->>Reporter: агрегированные данные (counts, time, details)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 минут Области требующие повышенного внимания:
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ 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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/core/internal/Классы/РепортерЛогПлоскийСписок.os (1)
16-23:ВывестиВЛогломается при выводе плана тестов (РезультатВыполнения = Неопределено)
ВывестиТестПланвызываетИсполнениеТестКонец(Тест, Неопределено), откуда вВывестиВЛогпопадаетРезультатВыполнения = Неопределено. В текущей реализации это приведёт к исключению при обращении кРезультатВыполнения.Состояние()/ОписанияПричин()и к статистике.Имеет смысл обработать сценарий плана тестов так же, как это сделано в
РепортерЛогДерево.ВывестиВЛог: приНеопределеновыводить только имя теста на уровнеИнформация, без состояния и времени.Предлагаемый фикс:
-Процедура ВывестиВЛог(Определение, РезультатВыполнения) +Процедура ВывестиВЛог(Определение, РезультатВыполнения = Неопределено) Поля = Новый КартаСоответствие; Поля.Вставить("ИмяТеста", Определение.ПолноеИмя()); - Уровень = СостоянияВыполненияТестов.УровеньЛогаСостояния(РезультатВыполнения.Состояние()); - - Причины = РезультатВыполнения.ОписанияПричин(); - Если Не Причины.Пусто() Тогда - Поля.Вставить("Причина", Причины); - КонецЕсли; - - Статистика = _РепортерСтатистика.СтатистикаТеста(Определение); - - Поля.Вставить("Результат", СостоянияВыполненияТестов.СимволСостояния(РезультатВыполнения.Состояние())); - Поля.Вставить( - "ВремяВыполнения", - Статистика.ПолучитьИлиУмолчание("ВремяЗавершения", 0) - - Статистика.ПолучитьИлиУмолчание("ВремяНачала", 0) - ); + Если РезультатВыполнения = Неопределено Тогда + Уровень = УровниЛога.Информация; + Иначе + Уровень = СостоянияВыполненияТестов.УровеньЛогаСостояния(РезультатВыполнения.Состояние()); + + Причины = РезультатВыполнения.ОписанияПричин(); + Если Не Причины.Пусто() Тогда + Поля.Вставить("Причина", Причины); + КонецЕсли; + + Статистика = _РепортерСтатистика.СтатистикаТеста(Определение); + + Поля.Вставить("Результат", СостоянияВыполненияТестов.СимволСостояния(РезультатВыполнения.Состояние())); + Поля.Вставить( + "ВремяВыполнения", + Статистика.ПолучитьИлиУмолчание("ВремяЗавершения", 0) - + Статистика.ПолучитьИлиУмолчание("ВремяНачала", 0) + ); + КонецЕсли;Так
ВывестиТестПланснова будет работать без падений, а боевое выполнение продолжит логироваться с состояниями и временем.Also applies to: 34-53
src/core/internal/Классы/РепортерJUnit.os (1)
116-135: Проверьте, чтоОписанияПричин()даёт удобочитаемую строку для атрибутаmessageДля
<testsuite>вы явно разворачиваетеПричины()черезПроцессорКоллекции().Обработать(...).ВСтроку(Символы.ПС), тогда как в<testcase>атрибутmessageсразу берётся изРезультат.ОписанияПричин(), который возвращаетСписокМассив. В зависимости от строкового представления коллекции это может дать менее удобное сообщение, чем явное соединение элементов.Можно рассмотреть вариант привести его к строке похожим образом, например:
ЗаписьXML.ЗаписатьАтрибут( "message", Результат.ОписанияПричин().ПроцессорКоллекции().ВСтроку(Символы.ПС) );чтобы поведение сообщения было однородным с остальными репортами.
Also applies to: 219-252
src/core/internal/Классы/АсинхронныйИсполнительТестов.os (2)
111-151: Тест выполняется даже при состояниях Прерван/Сломан послеПередКаждым— это противоречит модели состоянийСейчас вы останавливаете исполнение основного тела теста только если
РезультатВыполнения.Состояние = Ошибка:Если РезультатВыполнения.Состояние <> СостоянияВыполненияТестов.Ошибка Тогда ... вызов тестового метода ... КонецЕсли;При состояниях
ПрерванилиСломан, полученных вПередКаждым, основное тело теста всё равно выполняется, хотя логичнее было бы не выполнять тест вообще, как вы уже делаете на уровне набора (там проверки идут на равенствоУспех).Рекомендуем унифицировать поведение с уровнем набора и запускать тестовый метод только при полном успехе хуков, например:
Если РезультатВыполнения.Состояние = СостоянияВыполненияТестов.Успех Тогда ОбъединитьРезультатыВыполнения( РезультатВыполнения, ВыполнитьМетод( ТестНабор, Тест.ИмяМетода(), Тест.Параметры().ВМассив(), Тест.Таймаут() ) ); КонецЕсли;Так предположения/исключения в
ПередКаждымбудут действительно прерывать тест, а не только ухудшать итоговое состояние.Also applies to: 169-181, 197-207
351-379: ПроверкаРезультатВыполненияНабора.СостояниевПропуститьТестсейчас фактически не срабатываетВы добавили возможность пропускать тесты при плохом состоянии набора:
Если РезультатВыполненияНабора.Состояние <> ...Успех Тогда Результат.Пропустить = Истина; Результат.Причина = "Пропущен из-за состояния набора " + РезультатВыполненияНабора.Состояние; КонецЕсли;Но перед вызовом
ВыполнитьТестсостояние набора уже проверяется вИсполнить(запуск детей только приУспех), а внутриВыполнитьТествы нигде не обновляетеРезультатВыполненияНабора, поэтому его состояние остаётсяУспехна всём протяжении цикла по тестам. В итоге эта ветка либо никогда не сработает, либо сработает только если кто‑то начнёт мутироватьРезультатВыполненияНабораснаружи.Либо уберите эту проверку как мёртвый код, либо, если хотите действительно прекращать выполнение тестов после ухудшения состояния набора, начинайте объединять результаты отдельных тестов в
РезультатВыполненияНаборачерезОбъединитьРезультатыВыполненияи приводите условие в соответствие.
🧹 Nitpick comments (12)
src/cli/internal/Модули/КонсольноеПриложениеТестированиеСлужебный.os (1)
40-45: Логика корректна, но стоит добавить комментарий.Код правильно исключает импорт "oneunit" из собираемых зависимостей. Чтение следующей строки перед
Продолжитьнеобходимо для корректной работы цикла — без этого произошло бы зацикливание на одной и той же строке.Однако для улучшения читаемости и поддерживаемости кода рекомендуется добавить комментарий, объясняющий, почему "oneunit" обрабатывается особым образом (например, что фреймворк подключается отдельно через core-модуль).
Можно применить этот diff для добавления поясняющего комментария:
+ // Исключаем oneunit из собираемых импортов, т.к. фреймворк подключается отдельно через core-модуль Если СокрЛП(НРег(Импорт)) = "oneunit" Тогда Строка = ЧтениеТекста.ПрочитатьСтроку(); Продолжить; КонецЕсли;tests/fixtures/ПараметризованныйТест.os (1)
57-77: Покрытие всех состоянийСостоянияВыполненияТестовв параметризованном источнике согласованоСписок
Значенияи ожидание.Равно(5)согласуются друг с другом и с текущим набором состояний (Ошибка, Успех, Прерван, Пропущен, Сломан), что даёт хороший регрессионный тест на полноту обхода перечисления.На будущее можно убрать «магическое» число
5и, например, проверять.Равно(Значения.Количество()), чтобы ожидание автоматически подстраивалось при добавлении новых состояний, если вы захотите сделать тест менее жёстким к расширению перечисления.Also applies to: 90-93
src/core/internal/Классы/ЗаготовкаТестирование.os (1)
41-47: РазворачиваниеИнформацияОбОшибке.Причинаделает сообщение об ошибке более точнымПолучение
ИнформацияОбОшибке()и замена её наИнформацияОбОшибке.Причина(когда она заполнена) разумно убирает лишний уровень обёртки и позволяетПередВсемибросать «настоящую» причину падения конструктора тестового набора. Это улучшает диагностику и соответствует ожидаемому поведению.Опционально, для читаемости можно было бы использовать другое имя локальной переменной (не совпадающее с именем функции
ИнформацияОбОшибке()), но это стилистика и не критично.src/core/internal/Классы/РепортерOpenTestReport.os (1)
185-200: Фильтрация причин по типуИнформацияОбОшибкеуточняет формат, но может скрыть нетиповые причиныПереход к
Если ТипЗнч(Причина) <> Тип("ИнформацияОбОшибке") Тогда Продолжить;гарантирует, что вПодробноеОписаниеОшибкипередаётся ожидаемый тип и формат, что снижает риск падения репортера на «левом» значении.Стоит лишь убедиться, что все значимые причины в
Результат.Причины()действительно приходят какИнформацияОбОшибке; иначе нетиповые причины теперь будут молча игнорироваться в OpenTestReport.src/core/Модули/Предполагаем.os (1)
14-45: МодульПредполагаем: логика и интеграция сИсключениеТестПрерванвыглядят корректноПроверки в
ЧтоИстинно/ЧтоЛожнои реализацияПрерватьТестчерезИнформацияОбОшибке(Сообщение, Новый ИсключениеТестПрерван)хорошо вписываются в новую модель состоянияПрервани дают ожидаемое поведение допущений. Существенных проблем не вижу.При желании можно в будущем добавить перегрузки с сообщением по умолчанию, чтобы сократить шаблонный текст в тестах.
src/core/internal/Модули/СостоянияВыполненияТестов.os (1)
3-12: Состояния и приоритеты выглядят консистентно, можно чуть уменьшить дублированиеМаппинг состояний на уровни логов и приоритеты хорошо соответствует описанной модели (
Сломан/Ошибкакак самые тяжёлые,Прерван/Пропущенкак warning). Единственный небольшой момент — в блоках инициализации карт используются строковые литералы"Успех","Пропущен"и т.п., хотя рядом есть одноимённые экспортируемые переменные.Чтобы снизить риск расхождения при будущих правках, можно заменить литералы на сами переменные:
-ПриоритетыСостояний.Вставить("Успех", 0); +ПриоритетыСостояний.Вставить(Успех, 0); ... -СостоянияПриоритетов.Вставить(0, "Успех"); +СостоянияПриоритетов.Вставить(0, Успех);Это не функционально, но улучшает устойчивость к опечаткам.
Also applies to: 29-39, 57-67, 75-93
src/core/internal/Классы/РепортерЛогДерево.os (1)
9-10: Парковка/распарковка контейнеров аккуратно решает проблему пустых и сломанных наборовНовая схема с
ЗапаркованныйКонтейнериРаспарковатьКонтейнервыглядит корректной:
- для обычных наборов контейнер выводится один раз при первом тесте,
- для пустых или сломанных — при
ИсполнениеТестНаборКонецуже с итоговым состоянием и временем,
при этом префиксы иТекущийКонтейнеростаются консистентными.Существенных проблем не вижу. Максимум, можно добавить небольшой комментарий к
РаспарковатьКонтейнер, что параметрРезультатВыполненияреально используется только если контейнер ещё не был распаркован (кейс пустых/сломанных наборов) — это сделает логику понятнее следующему читателю.Also applies to: 57-66, 71-77, 131-168
src/core/internal/Классы/РезультатВыполненияТеста.os (1)
31-45: Обёртка для причин корректно обрабатывает как строки, так и объекты ошибокНовая функция в
ЗаменитьВсеоставляет строковые причины как есть и пускает остальные черезКраткоеОписаниеОшибки, что устраняет избыточное форматирование уже подготовленных сообщений и делает вывод более предсказуемым.Для читаемости можно переименовать параметр процедуры
ПриСозданииОбъектасСостояниена что‑то вродеНовоеСостояние, чтобы не перекрываться по имени с функциейСостояние(), но это исключительно косметика.src/core/internal/Классы/РепортерJUnit.os (1)
175-188: Деревья тестов не выводятся для неуспешных наборов — это может осложнить анализ пропусковУсловие
Если Результат.Состояние() = ...Успехприводит к тому, что для наборов в состояниях Пропущен/Прерван/Ошибка/Сломан вы видите только агрегированные счётчики и свойства набора, но не отдельные<testcase>. Это соответствует идее «не репортить тесты из сломанных наборов», но для пропущенных/прерванных наборов часть тулов предпочитает всё же видеть отдельные тесткейсы со статусом skipped.Если нужна более детальная картинка по пропущенным/прерванным тестам на уровне JUnit‑отчёта, можно ослабить условие (например, разрешить вывод детей для состояний Пропущен/Прерван) или сделать это настраиваемым.
src/core/internal/Классы/РепортерСтатистика.os (2)
13-15: Единый контейнерРезультатыи экспортируемыйРезультат(Определение)заметно упрощают интеграцию репортёровХранить результаты наборов и тестов в одной
КартаСоответствиеи раскрывать их черезРезультат(Определение)— хороший шаг к более чистому API, что видно по использованию вРепортерJUnitи других классах.Имеет смысл предусмотреть явную проверку на отсутствие ключа (например, выбрасывать осмысленное исключение), чтобы новые вызовы
Результат(...)не получали молчаНеопределенои не падали уже при обращении кСостояние()/Причины().Also applies to: 43-45, 59-60
183-231: Вывод ошибок теперь опирается на единый контейнерРезультатыФильтрация по состояниям Ошибка/Сломан и вывод заголовка с
ПолноеИмя()плюс подробных описаний изПодробноеОписаниеОшибкидают хороший человекочитаемый лог. Возможное дублирование сообщений для набора и его упавших тестов в логах не ломает функциональность и, наоборот, подчёркивает границу между проблемой набора и конкретными тестами.Если логи кажутся слишком многословными, можно со временем ограничить вывод, например, не логировать отдельно результат набора, когда все причины уже отражены в детях.
src/core/internal/Классы/АсинхронныйИсполнительТестов.os (1)
210-253: Классификация исключений вВыполнитьМетодаккуратно разводит Ошибка/Прерван/СломанОпора на
ТипЗнч(Причина.Параметры)с маппингом:
ИсключениеОшибкаУтверждения→Ошибка,ИсключениеТестПрерван→Прерван,- всё остальное →
Сломан
даёт нужное разделение, применимое ко всем вызовамВыполнитьМетод. Одновременно вы собираете вывод из консоли в отдельный поток, что позволяет потом честно отдать его вРезультатВыполненияТеста.Можно явно инициализировать
РезультатВыполненияМетода = НеопределенопередПопытка, чтобы не зависеть от поведения платформы при обращении к неинициализированной переменной в случае исключения (сейчас это не ломает логику, т.к. поле читается только приСостояние = Успех).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
README.md(4 hunks)docs/api/Модули/Предполагаем.md(1 hunks)src/cli/internal/Модули/КонсольноеПриложениеТестированиеСлужебный.os(1 hunks)src/cli/Классы/КомандаТестировать.os(1 hunks)src/core/internal/Классы/АсинхронныйИсполнительТестов.os(15 hunks)src/core/internal/Классы/ЗаготовкаТестирование.os(1 hunks)src/core/internal/Классы/ИсключениеТестПрерван.os(1 hunks)src/core/internal/Классы/РезультатВыполненияТеста.os(2 hunks)src/core/internal/Классы/РепортерGenericExecution.os(2 hunks)src/core/internal/Классы/РепортерJUnit.os(5 hunks)src/core/internal/Классы/РепортерOpenTestReport.os(5 hunks)src/core/internal/Классы/РепортерЛогДерево.os(4 hunks)src/core/internal/Классы/РепортерЛогПлоскийСписок.os(1 hunks)src/core/internal/Классы/РепортерСтатистика.os(8 hunks)src/core/internal/Модули/РезультатыВыполненияТестов.os(0 hunks)src/core/internal/Модули/СостоянияВыполненияТестов.os(1 hunks)src/core/Модули/Предполагаем.os(1 hunks)src/shared/Модули/ТестированиеСлужебный.os(0 hunks)tests/fixtures/НаборКрасныхТестов.os(1 hunks)tests/fixtures/НаборПадающийВКонструкторе.os(1 hunks)tests/fixtures/НаборПрерванный.os(1 hunks)tests/fixtures/НаборСВыключеннымиТестами.os(2 hunks)tests/fixtures/НаборСПрерваннымиТестами.os(1 hunks)tests/fixtures/ПараметризованныйТест.os(2 hunks)tests/Обнаружитель.os(2 hunks)tests/Предположения.os(1 hunks)
💤 Files with no reviewable changes (2)
- src/shared/Модули/ТестированиеСлужебный.os
- src/core/internal/Модули/РезультатыВыполненияТестов.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). (2)
- GitHub Check: test (dev) / build (ubuntu-latest, dev)
- GitHub Check: test (default) / build (ubuntu-latest, default)
🔇 Additional comments (27)
tests/fixtures/НаборКрасныхТестов.os (1)
1-17: LGTM!Тестовый набор правильно демонстрирует два состояния:
СостояниеОшибка: проверяет падение через утверждение (asserts)СостояниеСломан: проверяет падение через исключениеЭто хорошо согласуется с целями PR о разделении обработки ошибок утверждений и обычных исключений.
README.md (3)
124-150: Отличное улучшение документации!Четкое разделение между:
- Неперехваченными исключениями → тест помечается как
Сломанный- Ошибками утверждений (asserts) → тест помечается как
ОшибочныйДобавление примера с библиотекой asserts делает документацию более понятной и практичной.
280-304: LGTM! Хорошая документация механизма предположений.Раздел четко объясняет:
- Сценарии использования (когда нет смысла продолжать тест)
- Поведение при успехе и неудаче предположения
- Практические примеры для обычных и параметризованных тестов
Документация хорошо дополняет новую функциональность.
591-591: LGTM! Статистика обновлена для нового состояния "Прерван".Добавление счетчиков прерванных наборов и тестов логично дополняет новую систему состояний выполнения тестов.
Also applies to: 597-597
tests/fixtures/НаборПадающийВКонструкторе.os (1)
1-17: LGTM! Правильная структура фикстуры для тестирования падения конструктора.Фикстура демонстрирует сценарий, когда:
- Конструктор набора выбрасывает исключение
- Тесты из такого набора должны быть проигнорированы (не репортятся)
Говорящие имена тестов (
ЯПроигнорирован1/2/3) делают намерение понятным.src/core/internal/Классы/ИсключениеТестПрерван.os (1)
1-8: LGTM! Простая и корректная реализация исключения.Класс
ИсключениеТестПрерван:
- Правильно переопределяет представление объекта
- Имеет понятный тип "Тест прерван"
- Будет использоваться механизмом предположений для прерывания тестов
tests/Обнаружитель.os (1)
811-811: LGTM! Корректный переход на модуль состояний.Ожидаемые значения теперь берутся из модуля
СостоянияВыполненияТестов:
- Строка 811:
СостоянияВыполненияТестов.Успех- Строка 824:
СостоянияВыполненияТестов.ОшибкаЭто согласуется с рефакторингом на единую модель состояний выполнения тестов.
Also applies to: 824-824
tests/Предположения.os (2)
3-27: LGTM! Хороший тест для проверки ЧтоИстинно.Тест правильно проверяет оба сценария:
- Когда предположение истинно (Значение = Истина) → исключение не выбрасывается
- Когда предположение ложно (Значение = Ложь) → выбрасывается
ИсключениеТестПрерванПараметризация делает тест компактным и понятным.
29-66: LGTM! Полное тестирование механизма предположений.Тест
ЧтоЛожно:
- Правильно использует инверсию логики (строка 37)
- Проверяет оба сценария аналогично
ЧтоИстинноВспомогательная процедура
ПроверитьИсключение:
- Тщательно проверяет структуру исключения
- Валидирует тип, описание и параметры
- Подтверждает, что выброшено именно
ИсключениеТестПрерванsrc/cli/Классы/КомандаТестировать.os (1)
114-117: Нет необходимости включать состояние "Прерван" в условие завершения.Проверка показывает, что текущая реализация является намеренной и правильной:
- Прерванные тесты записываются в отдельные ключи статистики:
НаборовПрерваноиТестовПрервано- Состояние "Прерван" сгруппировано с "Пропущен" (строка 47 СостоянияВыполненияТестов.os), что указывает на его рассмотрение как успешного сценария
- Условие завершения намеренно проверяет только состояния ошибок и поломок (
НаборовОшибка,НаборовСломано,ТестовОшибка,ТестовСломано)- Это соответствует стандартным моделям отчётов о тестировании, где только ошибки/поломки вызывают код выхода 1
Текущая реализация корректна и не требует изменений.
tests/fixtures/НаборСВыключеннымиТестами.os (1)
67-67: Версионные шаблоны с пререлизами обработаны корректно.Проверка подтверждает, что используемые шаблоны соответствуют спецификации SemVer:
- Строка 67:
~2.0.0-rcкорректно включает совместимые пререлизные версии- Строка 123:
<2.0.0-rcкорректно ограничивает диапазон версиями, строго меньшими 2.0.0-rc, что обеспечивает исключение теста на поддерживаемых версиях ≥ 2.0.0Комментарий в строке 125 точно отражает намеренное поведение: тест не будет выполняться на требуемых версиях приложения.
docs/api/Модули/Предполагаем.md (2)
5-20: Документация дляЧтоИстинновыглядит завершённой и яснойОписание параметров и пример использования хорошо объясняют поведение и основной сценарий применения предположений.
24-39: Аналогично, раздел дляЧтоЛожночётко описывает контракт процедурыСигнатура, параметры и пример использования согласованы между собой и читаются однозначно.
tests/fixtures/НаборПрерванный.os (1)
1-16: Фикстура для прерванного набора хорошо моделирует целевой сценарий
Прерватьсячерез&ПередВсемии заведомо падающий тестНеДолженВыполнятьВПрерванномНаборедают чёткую проверку: если набор корректно помечается как прерванный, тело теста вообще не будет выполнено.tests/fixtures/НаборСПрерваннымиТестами.os (1)
1-25: Фикстура полноценно покрывает прерывание обычного, повторяемого и параметризованного тестовИнициализация
СчвПриСозданииОбъектаприХарактер = "Одиночка"и дальнейшее использование его вПрерванныйТестПовторяемыйдаёт наглядный сценарий прерывания по счётчику, а параметризованный тест проверяет аналогичную логику по значению параметра. Структура набора выглядит логичной и самодокументируемой.src/core/internal/Классы/РепортерOpenTestReport.os (1)
216-226: КартаСтатусыПоСостояниями использованиеНаихудшееСостояниевыглядят согласованными с моделью состоянийВыбор базового
Состояниепо статистике (сломано/ошибка/успех) и последующее вычислениеНаихудшееСостояние(Результат.Состояние(), Состояние)даёт ожидаемое агрегированное состояние для набора/теста. Маппинг вSUCCESSFUL/SKIPPED/ABORTED/FAILED/ERROREDчерезСтатусыПоСостояниямделает код компактным и легко расширяемым.Имеет смысл при изменении перечисления
СостоянияВыполненияТестовили обновлении версии схемы OpenTestReport помнить, что эту карту тоже нужно будет актуализировать, чтобы избежатьПолучить()на отсутствующем ключе.Also applies to: 252-257
src/core/internal/Классы/РепортерGenericExecution.os (1)
3-5: Маппинг состояний в XML и использование статистики в целом корректны, проверьте только единицыdurationНовый
_РепортерСтатистикаи использованиеСтатистикаТеста(Тест).ПолучитьИлиУмолчание("ВремяВыполнения", 0)для атрибутаdurationвыглядят логично, так же как и маппинг состояний:
Пропущен/Прерван→<skipped>,Ошибка→<failure>,Сломан→<error>, остальные — без дочернего элемента.Единственное, что стоит дополнительно проверить — что
ВремяВыполненияв статистике измеряется в тех же единицах, которых ожидает потребитель отчёта Generic Execution (мс vs. секунды и пр.), чтобы не получить искажённые цифры в отчётах.Если нужно перепроверить формат, можно посмотреть актуальную спецификацию Generic Execution и сравнить с тем, как сейчас рассчитывается
ВремяВыполненияв модулеРепортерСтатистика.Also applies to: 41-60, 67-70
src/core/internal/Классы/РезультатВыполненияТеста.os (1)
3-10: Переход на внутреннее поле_Состояниеи геттерСостояние()выглядит согласованнымЭкспортируемая функция
Состояние()и хранение во внутреннем_Состояниехорошо ложатся на новую модель состояний, внешнему коду менять ничего не нужно кроме имени геттера.src/core/internal/Классы/РепортерJUnit.os (1)
31-36: Статистика tests/skipped/failures/errors согласована с новой моделью состоянийИспользование
ТестовОбнаруженодляtests, суммыТестовПропущено + ТестовПрерванодляskippedи разнесениеТестовОшибка/ТестовСломанопоfailures/errorsхорошо отражает разделение «ошибка утверждения» vs «сломанный тест/инфраструктура». Аналогичная логика на уровне тест‑плана и отдельных наборов выглядит непротиворечиво.Also applies to: 95-101, 103-112
src/core/internal/Классы/РепортерСтатистика.os (3)
85-116: Расчёт состояния набора через статистику детей и собственный результат закрывает кейс «сломанных» наборовВ
ИсполнениеТестНаборКонецвы сначала определяете состояние набора по статистике детей (ТестовСломано/ТестовОшибка), а затем «ужесточаете» его черезНаихудшееСостояниес фактическим результатом набора. Это позволяет:
- учитывать как результаты отдельных тестов,
- так и падения в
ПередВсеми/ПослеВсехили конструкторе набора.
СчётчикиНаборовПропущено/Прервано/Ошибка/Сломано/Успешнозатем аккуратно собираются вСтатистикаТестПлана.
138-155: Учёт времён выполнения теста и разбор состояний вИсполнениеТестКонецвыглядит полнымЗапись
ВремяЗавершения,ДатаЗавершенияиВремяВыполненияв статистику теста, плюс помещение результата вРезультаты, даёт всё необходимое для репортинга. Разбор по состоянию (Успех/Пропущен/Прерван/Ошибка/Сломан) и явное исключение для неизвестного состояния помогут быстро отловить несогласованные изменения в модели состояний.Also applies to: 158-175
235-307: Статистика по наборам и тестам аккуратно расширена под новые состоянияДобавление
НаборовПрервано/НаборовОшибка (Ошибка+Сломано)и аналогичных полей для тестов, вычисление максимальной ширины чисел и формирование двух блоков статистики делают суммарный лог понятным. Ранний выход при отсутствии ошибок и режимеНичегосохраняет прежнее поведение, не засоряя лог.src/core/internal/Классы/АсинхронныйИсполнительТестов.os (5)
9-18: ПайплайнИсполнитькорректно выстраивает события и проверку конструкторов наборовПорядок:
ИсполнениеТестПланНачало,ИсполнениеТестНаборНачало,- попытка
ПропуститьНаборчерез отдельный метод,- затем
ПроверитьСозданиеНаборас публикациейИсполнениеТестНаборКонецпри падении конструктора
выглядит логичным и соответствует описанию PR («сломанные наборы» корректно помечаются, тесты из них не запускаются).Also applies to: 30-37, 54-73
80-107: Агрегация результатов хуков набора черезОбработатьСобытияНабораиОбъединитьРезультатыВыполнениявыглядит корректнойСтруктура
"Состояние, Причины, Вывод"с начальным состояниемУспехи последовательным объединением результатовПередВсеми/ПослеВсехчерезНаихудшееСостояниедаёт ожидаемое итоговое состояние набора и накапливает все причины и вывод без потерь.
272-325:Пропуститькорректно обрабатывает как исключения, так и явные пропускиЛогика:
- при состояниях
Ошибка/Сломан/Прерваниз методаПропустить*вы немедленно формируетеРезультатВыполненияТестас тем же состоянием и публикуете завершение;- при нормальном завершении смотрите на
РезультатВыполненияМетода.Пропуститьи, если он истина, формируете результат со состояниемПропущен.
Это хорошо разделяет «тест/набор действительно упал» и «тест/набор сознательно пропущен по условию».
381-390:ОбъединитьРезультатыВыполнениякорректно реализует принцип «наихудшее состояние + конкатенация причин/вывода»Использование
НаихудшееСостояниедля поляСостояние,ДобавитьВседляПричиныи простая конкатенацияВыводпозволяют безопасно аккумулировать результат по всей цепочке хуков/теста без потерь информации.
393-439:ПроверитьСозданиеНаборанадёжно покрывает падения и некорректный конструктор набораФункция:
- сначала выполняет метод
ТестНабори, если состояние неУспех, публикуетИсполнениеТестНаборКонецс соответствующим состоянием и причинами;- дополнительно считает набор «Сломан», если метод успешно отработал, но вернул
Неопределено.
Так вы защищаетесь как от явных исключений в конструкторе набора, так и от случаев, когда конструктор «молча» ничего не вернул, и при этом выполняете требование «тесты из сломанных наборов больше не репортятся».
* Замечания кролика
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/internal/Классы/РепортерЛогПлоскийСписок.os (1)
34-59: Критическая ошибка: РепортерЛогДерево.os использует несуществующий ключ статистикиВ файле
src/core/internal/Классы/РепортерСтатистика.osустанавливаются только ключи"ВремяНачала"и"ВремяЗавершения", но РепортерЛогДерево.os на строке 150 пытается получить ключ"ВремяВыполнения", которого нет. Это приведёт к тому, что время выполнения всегда будет равно 0 (значение по умолчанию).Необходимо исправить вычисление времени в РепортерЛогДерево.os (строки 148–151) на аналогичное РепортерЛогПлоскийСписок:
Поля.Вставить( "ВремяВыполнения", Статистика.ПолучитьИлиУмолчание("ВремяЗавершения", 0) - Статистика.ПолучитьИлиУмолчание("ВремяНачала", 0) );
🧹 Nitpick comments (4)
tests/Обнаружитель.os (2)
228-243: Отличный рефакторинг: замена дублирования на цикл.Использование цикла для проверки трёх тестов
ЯПроигнорирован1-3значительно улучшает читаемость и поддерживаемость кода. Индексация корректна (Сч - 1 для доступа к массиву).
478-493: Последовательный рефакторинг параметризованных тестов.Замена повторяющихся вызовов циклами для
ИсточникЗначение,ИсточникJSONЗначение,ИсточникJSONФайлиИсточникВыражениезначительно сокращает дублирование кода. Использование арифметических выраженийСч * 2 - 1иСч * 2для генерации ожидаемых параметров корректно.Also applies to: 546-561, 582-597, 700-715
src/core/internal/Классы/РепортерЛогПлоскийСписок.os (1)
43-52: Можно избежать повторного вызоваСостояние()за счёт локальной переменнойСейчас
РезультатВыполнения.Состояние()вызывается как минимум дважды — при вычислении уровня лога и символа состояния. Это мелочь, но можно упростить чтение и избежать дублирования:- Уровень = СостоянияВыполненияТестов.УровеньЛогаСостояния(РезультатВыполнения.Состояние()); + Состояние = РезультатВыполнения.Состояние(); + Уровень = СостоянияВыполненияТестов.УровеньЛогаСостояния(Состояние); ... - Поля.Вставить("Результат", СостоянияВыполненияТестов.СимволСостояния(РезультатВыполнения.Состояние())); + Поля.Вставить("Результат", СостоянияВыполненияТестов.СимволСостояния(Состояние));Так процедура станет чуть проще для чтения и сопровождения без изменения поведения.
src/core/internal/Классы/АсинхронныйИсполнительТестов.os (1)
392-401: Добавьте информативную причину для случая undefined.Когда конструктор набора возвращает
Неопределено, набор помечается как "Сломан", но список причин остаётся пустым (Списки.ИзЭлементов()). Рекомендуется добавить информативное сообщение в причины, чтобы упростить отладку.Примените этот diff для добавления информативной причины:
ИначеЕсли Результат.РезультатВыполненияМетода = Неопределено Тогда + ИнформацияОшибки = Новый ИнформацияОбОшибке( + "Конструктор тестового набора не вернул объект", + , + Неопределено + ); + Результат = _Поделка.НайтиЖелудь( "РезультатВыполненияТеста", Массивы.ИзЭлементов( СостоянияВыполненияТестов.Сломан, - Списки.ИзЭлементов(), + Списки.ИзЭлементов(ИнформацияОшибки), Результат.Вывод ) );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/core/internal/Классы/АсинхронныйИсполнительТестов.os(15 hunks)src/core/internal/Классы/РепортерЛогПлоскийСписок.os(2 hunks)src/core/internal/Модули/СостоянияВыполненияТестов.os(1 hunks)tests/Обнаружитель.os(14 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/internal/Модули/СостоянияВыполненияТестов.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). (1)
- GitHub Check: test (dev) / build (ubuntu-latest, dev)
🔇 Additional comments (16)
tests/Обнаружитель.os (8)
19-19: Обновление ожидаемого количества наборов соответствует добавленным фикстурам.Изменение с предыдущего значения на 14 корректно отражает добавление новых тестовых наборов в рамках данного PR (НаборПрерванный, НаборСПрерваннымиТестами, НаборПадающийВКонструкторе, Тест и обновлённый НаборКрасныхТестов).
618-637: Корректная проверка параметризации системным перечислением.Использование цикла
Для Каждогодля итерации по значениямСтатусСообщенияс ручным управлением счётчиком корректно проверяет генерацию параметризованных тестов из системных перечислений.
656-679: Динамическая проверка пользовательского перечисления через рефлексию.Использование рефлексии для итерации по свойствам
СостоянияВыполненияТестовпозволяет проверить параметризацию пользовательскими перечислениями. Ожидание ровно 5 состояний (строка 656) создаёт жёсткую привязку к структуре перечисления, что является преднамеренным — тест должен упасть при изменении количества состояний.
758-773: Проверка повторяемых тестов корректна.Циклическая проверка повторяемых тестов
ТриРазаиТриРазаИменнойс соответствующими отображаемыми именами корректно валидирует функциональность повторения тестов.Also applies to: 794-809
813-847: Добавлена проверка именованного повторяемого теста.Новый раздел
ТриРазаИменнойИменованныйкорректно проверяет комбинацию аннотацииПовторяемыйсОтображаемоеИмя, что соответствует целям PR по улучшению работы с именами тестов.
849-895: Новые тестовые наборы полностью соответствуют целям PR.Добавленные наборы тестируют ключевые новые возможности:
Тест: базовая проверка простых тестовНаборПрерванный: прерывание набора вПередВсеми(механизм предположений)НаборСПрерваннымиТестами: прерывание отдельных тестов (простых, повторяемых, параметризованных)НаборПадающийВКонструкторе: обработка падения в конструкторе с флагомПроверятьТестНабор = ЛожьЛогика проверок согласована с целями PR о переработке состояний и обработке сломанных наборов.
Also applies to: 897-930, 932-1030, 1032-1071
1073-1119: Обновлённые имена тестов отражают новую систему состояний.Переименование тестов в
СостояниеОшибкаиСостояниеСломанявно отражает переработанную логику разделения состояний: "Ошибка" для падений с ошибками утверждений vs "Сломан" для других исключений, что соответствует описанию PR.
1125-1141: Расширение функции для проверки сломанных наборов.Добавление параметра
ПроверятьТестНаборс условной логикой проверки объекта набора позволяет корректно тестировать наборы, падающие в конструкторе. Для таких наборов ожидаетсяНеопределеновместо экземпляра объекта, что соответствует описанию PR о корректной обработке сломанных наборов.src/core/internal/Классы/АсинхронныйИсполнительТестов.os (8)
30-32: LGTM! Корректная валидация создания набора.Новая проверка
ПроверитьСозданиеНаборасоответствует целям PR: наборы, которые выбрасывают исключение в конструкторе, корректно помечаются как сломанные и не выполняются.
35-36: LGTM! Последовательная миграция на систему состояний.Переход от "Результат" к "Состояние" и от "РезультатыВыполненияТестов" к "СостоянияВыполненияТестов" применён последовательно во всех структурах файла.
Also applies to: 62-62, 83-84, 147-148, 200-200
46-52: LGTM! Корректная логика пропуска тестов из сломанных наборов.Новая логика выполняет тесты только при успешном состоянии (после ПередВсеми). Это соответствует целям PR: тесты из сломанных наборов не выполняются. Важно отметить, что ПослеВсех (lines 54-57) всё равно выполняется для корректной очистки ресурсов.
277-279: LGTM! Улучшена унификация публикации событий.Использование переменной
ИмяСобытиявместо дублирования строковых констант улучшает читаемость и снижает риск ошибок.Also applies to: 297-297, 316-316
282-298: LGTM! Корректная обработка ошибок при проверке условий пропуска.Логика корректно разделяет два случая:
- Ошибки/поломки при проверке условий (публикуется соответствующее состояние)
- Нормальный пропуск на основе условий (обрабатывается в else-ветке)
305-312: LGTM! Корректная обработка пропущенных тестов.Пропуск теперь корректно обрабатывается как состояние общего цикла с сохранением причины пропуска. Это соответствует целям PR.
357-367: Исходный комментарий рецензии неверен — функция существует и работает корректно.Проверка подтвердила, что функция
НаихудшееСостояниеопределена в модулеСостоянияВыполненияТестов.os(строка 57), правильно экспортирована и полностью инициализирована. Обе приоритетные карты содержат все пять требуемых состояний (Успех, Пропущен, Прерван, Ошибка, Сломан) с корректными приоритетами. Функция правильно определяет наихудшее состояние из двух входных значений. Код не требует исправлений.
21-21: Переименование API проверено и корректно.Функция
ПропуститьНаборуспешно переименована вПропуститьОпределениебез нарушения внутреннего кода. Проверка показала:
- Старое имя
ПропуститьНаборотсутствует в файле и не используется в проекте (0 результатов)- Новое имя корректно объявлено в экспортах (строки 21, 115) и определено (строка 328)
- Класс находится в
internalпакете (src/core/internal/), предназначенном для внутреннего использования- Логика обхода родительской цепи безопасна — метод
Родитель()существует во всех типах определений- Никаких оставшихся ссылок на старый API не найдено
Изменение полностью внедрено и не вызывает конфликтов в кодовой базе.
There was a problem hiding this comment.
🧩 Analysis chain
Проверьте определение типов исключений.
Новая логика классификации ошибок корректно разделяет ошибки утверждений, прерванные тесты и сломанные тесты. Убедитесь, что типы ИсключениеОшибкаУтверждения и ИсключениеТестПрерван определены и доступны в контексте выполнения.
Выполните следующий скрипт для проверки определения этих типов:
🏁 Script executed:
#!/bin/bash
# Поиск определений типов исключений
rg -n --type-add 'oscript:*.os' --type oscript 'ИсключениеОшибкаУтверждения|ИсключениеТестПрерван' -C 3Length of output: 1577
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for class definitions of exception types
rg -n 'Класс.*Исключение' --type-add 'oscript:*.os' --type oscript -A 5
Length of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for where exceptions are thrown (ВызватьИсключение)
# and look for assertion-related exception creation
rg -n 'ВызватьИсключение' --type-add 'oscript:*.os' --type oscript -B 2 -A 2 | head -100
Length of output: 6275
---
🏁 Script executed:
```shell
#!/bin/bash
# Find Ожидаем module and check assertion failure handling
find . -name "*Ожидаем*" -type f 2>/dev/null
rg -n 'Ожидаем' --type-add 'oscript:*.os' --type oscript | grep -i 'модуль\|Функция\|Процедура' | head -20
Length of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# List all .os files and search for assertion/ошибка handling
fd -e os src/core | head -30
echo "---"
# Search more broadly for exception info handling
rg -n 'ИнформацияОбОшибке' --type-add 'oscript:*.os' --type oscript -B 3 -A 3
Length of output: 8506
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for Ожидаем-related assertions and exception creation
fd . --type f -e os | xargs grep -l "Ожидаем\|ИсключениеОшибкаУтверждения" 2>/dev/null | head -10
echo "---"
# Specifically search for where assertions might fail
rg -n 'Ожидаем\.' --type-add 'oscript:*.os' --type oscript -c | head -10
Length of output: 709
---
🏁 Script executed:
```shell
#!/bin/bash
# Find and examine Ожидаем module specifically
fd . --type f -name "*Ожидаем*" -o -name "*Assertion*" 2>/dev/null
echo "---"
# Search for where assertion failures are thrown
rg -n 'Ожидаем\.Что|ВызватьИсключение.*Ошибка' --type-add 'oscript:*.os' --type oscript src/core | head -15
Length of output: 197
Обработка типа исключения ИсключениеОшибкаУтверждения недостижима.
Проверка на строке 233 никогда не будет истинной — тип ИсключениеОшибкаУтверждения не выбрасывается нигде в коде. Обработка для ИсключениеТестПрерван корректна, но условие для ошибок утверждений (line 233-234) — это мёртвый код. Уточните, какой тип исключения выбрасывает библиотека Ожидаем при ошибке утверждения, и обновите условие соответственно.
🤖 Prompt for AI Agents
In src/core/internal/Классы/АсинхронныйИсполнительТестов.os around lines
233–239, the branch checking for ИсключениеОшибкаУтверждения is dead because
that exception type is never thrown by the Ожидаем/assertion library; update the
condition to the actual exception type used by Ожидаем (or a common base type)
so assertion failures are classified as СостоянияВыполненияТестов.Ошибка. To
fix: inspect the Ожидаем library (search for thrown exception class names or
check docs) to identify the real exception type for assertion failures, replace
ИсключениеОшибкаУтверждения with that type (or add a type
mapping/contains-message check or use a base exception) and remove any
unreachable branch if unnecessary.
There was a problem hiding this comment.
Кролик, этот тип будет выбрасываться библиотекой asserts, в пул реквесте: oscript-library/asserts#15 как раз это реализовано, и этот пул реквест зависим от пул реквеста в asserts
There was a problem hiding this comment.
@sfaqer, спасибо за уточнение! Понял — этот PR готовит обработку исключений для новой версии библиотеки asserts, которая добавит тип ИсключениеОшибкаУтверждения в oscript-library/asserts#15.
Такой подход вполне корректен: код здесь является forward-compatible и будет работать корректно после выхода соответствующей версии asserts. Зависимость уже упомянута в описании PR, так что всё в порядке.
Снимаю замечание — реализация правильная, просто опережает релиз зависимости. 👍
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/core/internal/Классы/АсинхронныйИсполнительТестов.os (1)
369-421: Рассмотрите более явную обработку ошибки инстанцирования.Функция корректно проверяет создание набора, однако обработка случая
РезультатВыполненияМетода = Неопределено(строки 392-416) использует паттерн с искусственным созданием исключения. Рассмотрите возможность более явной обработки или улучшения сообщения об ошибке для большей ясности.Возможное улучшение:
Попытка - ВызватьИсключение "Не удаётся инстанцировать тестовый набор"; + ВызватьИсключение "Конструктор тестового набора вернул неопределённое значение"; Исключение ИнформацияОбОшибке = ИнформацияОбОшибке(); КонецПопытки;src/core/internal/Классы/РепортерJUnit.os (1)
217-217: Рассмотрите заполнение атрибутаtypeдля улучшения информативности.Атрибут
typeзаписывается как пустая строка. Для улучшения информативности отчёта можно заполнить его типом исключения или названием состояния (например, "AssertionError" для ошибок или "Exception" для сломанных тестов).Пример:
- ЗаписьXML.ЗаписатьАтрибут("type", ""); + Если Результат.Состояние() = СостоянияВыполненияТестов.Ошибка Тогда + ЗаписьXML.ЗаписатьАтрибут("type", "AssertionError"); + Иначе + ЗаписьXML.ЗаписатьАтрибут("type", "Exception"); + КонецЕсли;src/core/internal/Классы/РепортерСтатистика.os (3)
104-125: Сложная логика определения состояния набора — рассмотрите упрощение.Логика определения итогового состояния тестового набора проверяет статистику дочерних тестов (строки 105-111), затем объединяет её с собственным состоянием набора через
НаихудшееСостояние()(строка 113). Хотя логика корректна, она может быть не очевидна для последующего сопровождения.Рассмотрите добавление комментария, поясняющего порядок обработки:
- Сначала определяется агрегированное состояние по дочерним тестам
- Затем оно объединяется с собственным состоянием набора (например, исключение в конструкторе)
- Берётся наихудшее из двух состояний
181-181: Улучшите сообщение об ошибке для отладки.Сообщение об исключении не содержит информации о фактическом состоянии, что усложнит диагностику проблемы.
Примените это исправление:
- ВызватьИсключение "Неизвестное состояние выполнения теста"; + ВызватьИсключение СтрШаблон("Неизвестное состояние выполнения теста: %1", Результат.Состояние());
264-272: Рассмотрите более точную формулировку для объединённого счётчика ошибок.Переменные
НаборовОшибкаиТестовОшибкаобъединяют как состояние "Ошибка", так и "Сломан", но в сообщении статистики (строки 286, 299) используется формулировка "ошибочных", которая не полностью отражает включение сломанных элементов.Для большей ясности можно:
- Вывести раздельно: "Наборов ошибка" и "Наборов сломано"
- Или использовать более точную формулировку: "Наборов с ошибками" или "Наборов неуспешных"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/core/internal/Классы/АсинхронныйИсполнительТестов.os(15 hunks)src/core/internal/Классы/РепортерJUnit.os(5 hunks)src/core/internal/Классы/РепортерЛогПлоскийСписок.os(2 hunks)src/core/internal/Классы/РепортерСтатистика.os(8 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). (1)
- GitHub Check: test (default) / build (ubuntu-latest, default)
🔇 Additional comments (13)
src/core/internal/Классы/РепортерЛогПлоскийСписок.os (2)
39-55: Рефакторинг логики вывода верифицирован и готов к использованию.Все проверки пройдены успешно:
Модуль
СостоянияВыполненияТестовсуществует и содержит оба необходимых метода (экспортированные):
УровеньЛогаСостояния()(строка 43) — возвращает уровень лога на основе состоянияСимволСостояния()(строка 13) — возвращает символ на основе состоянияМетоды корректно вызываются в
РепортерЛогПлоскийСписок.osс правильными аргументами (строки 43, 52)Удалённые обработчики и функции не имеют ссылок в кодовой базе и безопасно удалены
34-34: Исходное замечание некорректно: процедура является внутренней и не экспортируется.Процедура
ВывестиВЛогне имеет модификатораЭкспорт, что означает, что она закрытая и не может быть вызвана внешними потребителями. Единственный вызов в кодовой базе (строка 29) уже передаёт оба параметра явно. Изменение сигнатуры не создаёт проблем совместимости.Likely an incorrect or invalid review comment.
src/core/internal/Классы/АсинхронныйИсполнительТестов.os (9)
17-17: Событие корректно публикуется перед проверками.Публикация события начала набора перед проверками пропуска и создания корректна, так как соответствующие функции
ПропуститьиПроверитьСозданиеНаборасамостоятельно публикуют завершающее событие при необходимости.
30-32: LGTM!Проверка создания набора перед его выполнением корректно реализует требование о пометке наборов со сломанными конструкторами.
34-52: LGTM!Переход на модель состояний реализован корректно. Логика пропуска тестов при ошибках в
ПередВсеми()и безусловное выполнениеПослеВсех()соответствуют стандартному поведению тестовых фреймворков.
111-112: LGTM!Публикация события начала теста выполнена корректно.
214-239: LGTM!Классификация исключений реализована корректно. Разделение на ошибки утверждений (
ИсключениеОшибкаУтверждения), прерванные тесты (ИсключениеТестПрерван) и сломанные тесты соответствует описанию PR. Как уже обсуждалось, код является forward-compatible с зависимостью от oscript-library/asserts#15.
328-355: LGTM!Рефакторинг функции
ПропуститьОпределениевыполнен корректно. Упрощённая сигнатура и экспорт функции соответствуют новой модели работы с состояниями.
272-326: LGTM!Функция
Пропуститькорректно реализует логику пропуска с использованием модели состояний. Обработка различных состояний (Ошибка, Сломан, Прерван, Пропущен) и публикация соответствующих событий выполнены правильно.
34-39: Рефакторинг модели состояний выполнен последовательно.Переход от "Результат" к "Состояние" реализован единообразно во всех структурах результатов выполнения. Использование общей структуры
(Состояние, Причины, Вывод)для наборов, тестов и методов обеспечивает согласованность кода.Also applies to: 82-87, 146-151, 246-252
357-367: МетодНаихудшееСостояниеопределён и доступен — подтверждение пройдено.Проверка показала, что метод
СостоянияВыполненияТестов.НаихудшееСостояние()корректно определён в файлеsrc/core/internal/Модули/СостоянияВыполненияТестов.os(строка 57) как экспортируемая функция и доступен для использования. Его применение в процедуреОбъединитьРезультатыВыполнения(строки 359–362) соответствует ожидаемой сигнатуре. Проблем не обнаружено.src/core/internal/Классы/РепортерJUnit.os (2)
31-35: Корректное объединение состояний для JUnit формата.Объединение пропущенных и прерванных тестов в атрибут
skippedсоответствует семантике JUnit, где оба состояния трактуются как пропуски.
164-177: Вопрос о выводе дочерних тестов для пропущенных наборов.TODO-комментарий поднимает важный вопрос: следует ли выводить дочерние тесты, если тестовый набор пропущен или прерван? Текущая реализация выводит дочерние элементы только для успешных наборов (строка 165), что может скрывать информацию о тестах внутри пропущенных наборов.
Рассмотрите, соответствует ли это ожидаемому поведению для вашего случая использования. Для полноты отчётности может быть полезно выводить тесты и в случае пропуска набора.
There was a problem hiding this comment.
🧩 Analysis chain
Проверьте безопасность двойного вызова .Получить().
Двойной вызов .Получить() предполагает, что Результаты.Получить(Определение) возвращает обёртку (например, Optional/Maybe из fluent), у которой есть метод .Получить(). Если ключ отсутствует в Результаты, первый вызов вернёт Неопределено, что приведёт к ошибке при попытке вызова .Получить() на Неопределено.
Убедитесь, что все вызовы Результат() происходят после того, как соответствующий элемент был добавлен в Результаты (строки 102, 166), или добавьте проверку на существование ключа.
🏁 Script executed:
#!/bin/bash
# Проверка использования Результат() и гарантии что ключи существуют
rg -n -A3 -B3 'Результат\(' --type-add 'os:*.os' --type osLength of output: 1447
Now I need to examine the actual code to see how Результаты is declared and used.
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Find the РепортерСтатистика.os file and examine Результаты structure
cat src/core/internal/Классы/РепортерСтатистика.os | head -100Length of output: 3049
[/run_scripts]
---
🏁 Script executed:
```shell
#!/bin/bash
# Find where Результаты is populated and used
cat src/core/internal/Классы/РепортерСтатистика.os | grep -n "Результаты"
Length of output: 280
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Get context around lines 102 and 166
sed -n '95,110p' src/core/internal/Классы/РепортерСтатистика.os
echo "---"
sed -n '160,175p' src/core/internal/Классы/РепортерСтатистика.osLength of output: 1219
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Check context around where Результат() is called in РепортерJUnit.os
sed -n '95,120p' src/core/internal/Классы/РепортерJUnit.os
echo "---"
sed -n '190,210p' src/core/internal/Классы/РепортерJUnit.osLength of output: 1490
Двойной вызов .Получить() использует небезопасный паттерн и должен быть исправлен.
Функция Результат() (строки 43-46) нарушает паттерны, используемые в остальной части файла. Методы СтатистикаНабора() и СтатистикаТеста() (строки 26-36) безопасно обрабатывают отсутствие ключей через .ВычислитьЕслиОтсутствует(), тогда как Результат() использует прямой доступ через .Получить(). Если ключ отсутствует, КартаСоответствие.Получить() вернёт Неопределено, и вызов .Получить() на нём вызовет ошибку.
Используйте безопасный паттерн, согласованный с остальным кодом:
Функция Результат(Определение) Экспорт
Возврат Результаты
.ВычислитьЕслиОтсутствует(Определение, "К -> Новый КартаСоответствие")
.Получить();
КонецФункции🤖 Prompt for AI Agents
In src/core/internal/Классы/РепортерСтатистика.os around lines 43–46, replace
the unsafe double .Получить() call with the same safe pattern used elsewhere:
first call Результаты.ВычислитьЕслиОтсутствует(Определение, <factory that
returns Новый КартаСоответствие>) to ensure a map exists for the key, then call
.Получить() on that guaranteed value and return it; this prevents a possible
undefined value and runtime error when the key is missing.
There was a problem hiding this comment.
Кролик, первый получить возвращает Опциональный, второй получить либо вернёт значение которое требуется либо выдаст исключение в случае если значения не было. Небезопасный паттерн тут не подходит т.к нет дефолтного значения, если к нам пришли за результатом то результат должен быть, во всех местах кодовой базы туда идёт обращение только для тестов\наборов которые выполнялись.
There was a problem hiding this comment.
@sfaqer, спасибо за разъяснение! Теперь понимаю — первый .Получить() возвращает Опциональный из библиотеки fluent, а второй либо извлекает значение, либо выбрасывает исключение при его отсутствии. Это действительно правильный подход, когда результат должен существовать по контракту использования функции. Извините за некорректное замечание.
✏️ Learnings added
Learnt from: sfaqer
Repo: sfaqer/OneUnit PR: 15
File: src/core/internal/Классы/РепортерСтатистика.os:43-46
Timestamp: 2025-11-20T00:59:17.157Z
Learning: В файле src/core/internal/Классы/РепортерСтатистика.os паттерн `Результаты.Получить(Определение).Получить()` является корректным: первый `.Получить()` возвращает тип `Опциональный` из библиотеки fluent, второй `.Получить()` распаковывает значение или выбрасывает исключение при отсутствии. Функция `Результат(Определение)` вызывается только для тестов/наборов, которые были выполнены, поэтому исключение при отсутствии значения является желаемым поведением.
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.
There was a problem hiding this comment.
Кролик, Опциональный не из библиотеки fluent, он из библиотеки Optional
There was a problem hiding this comment.
@sfaqer, спасибо за уточнение! Вы правы — Опциональный приходит из библиотеки Optional, а не fluent. Обновил информацию в памяти.
✏️ Learnings added
Learnt from: sfaqer
Repo: sfaqer/OneUnit PR: 15
File: src/core/internal/Классы/РепортерСтатистика.os:43-46
Timestamp: 2025-11-20T01:01:51.093Z
Learning: В файле src/core/internal/Классы/РепортерСтатистика.os паттерн `Результаты.Получить(Определение).Получить()` является корректным: первый `.Получить()` возвращает тип `Опциональный` из библиотеки Optional, второй `.Получить()` распаковывает значение или выбрасывает исключение при отсутствии. Функция `Результат(Определение)` вызывается только для тестов/наборов, которые были выполнены, поэтому исключение при отсутствии значения является желаемым поведением.
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.
⛔ Learnings removed
Learnt from: sfaqer
Repo: sfaqer/OneUnit PR: 15
File: src/core/internal/Классы/РепортерСтатистика.os:43-46
Timestamp: 2025-11-20T00:59:17.157Z
Learning: В файле src/core/internal/Классы/РепортерСтатистика.os паттерн `Результаты.Получить(Определение).Получить()` является корректным: первый `.Получить()` возвращает тип `Опциональный` из библиотеки fluent, второй `.Получить()` распаковывает значение или выбрасывает исключение при отсутствии. Функция `Результат(Определение)` вызывается только для тестов/наборов, которые были выполнены, поэтому исключение при отсутствии значения является желаемым поведением.
🧠 Learnings used
Learnt from: sfaqer
Repo: sfaqer/OneUnit PR: 15
File: src/core/internal/Классы/РепортерСтатистика.os:43-46
Timestamp: 2025-11-20T00:59:17.157Z
Learning: В файле src/core/internal/Классы/РепортерСтатистика.os паттерн `Результаты.Получить(Определение).Получить()` является корректным: первый `.Получить()` возвращает тип `Опциональный` из библиотеки fluent, второй `.Получить()` распаковывает значение или выбрасывает исключение при отсутствии. Функция `Результат(Определение)` вызывается только для тестов/наборов, которые были выполнены, поэтому исключение при отсутствии значения является желаемым поведением.
* Исправлена ошибка постоянной сортировке детей тест набора
* Добавил настройки для запуска тестов через bslls
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.bsl-language-server.json(1 hunks)lib.config(1 hunks)
🔇 Additional comments (3)
lib.config (1)
28-29: Конфигурация модуля Предполагаем добавлена корректно.Проверка подтвердила:
- Файл модуля
src/core/Модули/Предполагаем.osсуществует и содержит три процедуры- Две публичные экспортируемые процедуры:
ЧтоИстинноиЧтоЛожно- Одна внутренняя вспомогательная процедура:
ПрерватьТест(без модификатора Экспорт)- Запись в
lib.config(строки 28-29) синтаксически корректна и правильно размещена- Документация и тесты согласованы с конфигурацией
.bsl-language-server.json (2)
9-9: Верифицируйте regex паттерн для парсинга результатов.Regex паттерн
^[^.]+\\.([^\\s]+).*$предназначен для извлечения имени теста. Требуется убедиться, что этот паттерн корректно извлекает имена тестов из формата вывода, генерируемого новым механизмом состояний и рефакторингом, описанным в PR.Подтвердите, что:
- Паттерн соответствует формату вывода команд
d(discovery) иe(execution);- Захватываемая группа
([^\\s]+)корректно извлекает имена тестов в новой схеме состояний.
6-8: Относительные пути используются корректно и соответствуют документированному поведению.BSL Language Server разрешает относительные пути из корневой директории проекта / рабочей области, где запускается языковой клиент. Документация проекта подтверждает, что относительный путь разрешается относительно текущего каталога, где запускаются тесты, обычно это корень проекта.
Рабочие потоки GitHub проекта используют идентичный паттерн с относительными путями, и конфигурационный пример в документации также следует этому же паттерну. Никаких ошибок или проблем с разрешением путей в кодовой базе не обнаружено.
Текущая конфигурация корректна и соответствует документированным практикам.
Likely an incorrect or invalid review comment.
| "getTestsArguments": "./src/cli/main.os d --file %s --mode flat", | ||
| "runAllTestsArguments": " ./src/cli/main.os e --file %s --genericCoverage build/genericCoverage.xml --openTestReport build/otr.xml --cobertura build/cobertura.xml", | ||
| "runTestArguments": " ./src/cli/main.os e --file %s -m %s --genericCoverage build/genericCoverage.xml --openTestReport build/otr.xml --cobertura build/cobertura.xml", |
There was a problem hiding this comment.
Устраните несогласованность в пробелах перед аргументами.
Строка 6 (getTestsArguments) начинается без пробела, тогда как строки 7-8 (runAllTestsArguments, runTestArguments) начинаются с пробела. Это несогласованность может привести к неправильному парсингу команды в зависимости от того, как строки объединяются с путём или другими параметрами.
Рекомендуется унифицировать формат:
- "runAllTestsArguments": " ./src/cli/main.os e --file %s --genericCoverage build/genericCoverage.xml --openTestReport build/otr.xml --cobertura build/cobertura.xml",
- "runTestArguments": " ./src/cli/main.os e --file %s -m %s --genericCoverage build/genericCoverage.xml --openTestReport build/otr.xml --cobertura build/cobertura.xml",
+ "runAllTestsArguments": "./src/cli/main.os e --file %s --genericCoverage build/genericCoverage.xml --openTestReport build/otr.xml --cobertura build/cobertura.xml",
+ "runTestArguments": "./src/cli/main.os e --file %s -m %s --genericCoverage build/genericCoverage.xml --openTestReport build/otr.xml --cobertura build/cobertura.xml",📝 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.
| "getTestsArguments": "./src/cli/main.os d --file %s --mode flat", | |
| "runAllTestsArguments": " ./src/cli/main.os e --file %s --genericCoverage build/genericCoverage.xml --openTestReport build/otr.xml --cobertura build/cobertura.xml", | |
| "runTestArguments": " ./src/cli/main.os e --file %s -m %s --genericCoverage build/genericCoverage.xml --openTestReport build/otr.xml --cobertura build/cobertura.xml", | |
| "getTestsArguments": "./src/cli/main.os d --file %s --mode flat", | |
| "runAllTestsArguments": "./src/cli/main.os e --file %s --genericCoverage build/genericCoverage.xml --openTestReport build/otr.xml --cobertura build/cobertura.xml", | |
| "runTestArguments": "./src/cli/main.os e --file %s -m %s --genericCoverage build/genericCoverage.xml --openTestReport build/otr.xml --cobertura build/cobertura.xml", |
🤖 Prompt for AI Agents
.bsl-language-server.json lines 6-8: there is an inconsistency in leading
whitespace for the command argument strings — line 6 (getTestsArguments) has no
leading space while lines 7-8 (runAllTestsArguments, runTestArguments) start
with a space; remove the leading space from runAllTestsArguments and
runTestArguments so all three entries start consistently (no leading space) and
verify there are no unintended trailing spaces.
sfaqer
force-pushed
the
feature/assumptions
branch
from
1a9f53e to
ef28ccb
Compare
sfaqer
force-pushed
the
feature/assumptions
branch
from
ef28ccb to
bbeb7ef
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/fixtures/НаборСПрерваннымиТестами.os (1)
1-4: Глобальное состояние и инициализация счётчикаИспользование
Перем Сч;вместе с установкой вПриСозданииОбъектавыглядит уместно для проверки поведения набора‑одиночки. Если в будущем фикстуру будут запускать вне OneUnit‑раннера (или с другим жизненным циклом), можно опционально подстраховаться инициализацией по умолчанию:Если Сч = Неопределено Тогда Сч = 0; КонецЕсли;Сейчас это скорее не обязательно, так как фикстура явно заточена под корректный вызов
ПриСозданииОбъекта.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/fixtures/НаборПрерванный.os(1 hunks)tests/fixtures/НаборСПрерваннымиТестами.os(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/fixtures/НаборПрерванный.os
🔇 Additional comments (3)
tests/fixtures/НаборСПрерваннымиТестами.os (3)
5-8: Базовый пример прерванного теста выглядит корректным
ПрерванныйТестминимально и наглядно демонстрирует сценарий прерывания черезПредполагаем.ЧтоИстинно(Ложь, ...). Для целей фикстуры (проверка нового состояния «Прерван») реализация достаточная и читаемая.
10-20: Сценарии для повторяемого и параметризованного прерванных тестов покрывают нужные кейсы
- Повторяемый тест с
&ПовторяемыйТест(4)и проверкойСч <= 2даёт первые два успешных прохода и два прерывания, что хорошо тестирует статистику и агрегацию состояний.- Параметризованный тест с
ИсточникJSONи условиемЗначение <= 2аналогично создаёт комбинацию «пройден / прерван» на данных.Это удачный набор кейсов для проверки новой логики «Предположений» на повторяемых и параметризованных тестах. Главное при доработке юнит‑тестов вокруг этой фикстуры — явно проверить, что такие падения действительно маркируются как «Прерван», а не «Сломан»/«Ошибка», и что статистика по итерациям и параметрам сходится с ожиданиями.
22-26: Инициализация счётчика в наборе‑одиночке согласована с поведением тестовАннотация
&ТестовыйНабор(Характер = "Одиночка")совместно сПриСозданииОбъекта, гдеСчсбрасывается в 0, хорошо согласуется с идеей считать все повторенияПрерванныйТестПовторяемыйв пределах одного объекта набора. Это делает поведение счётчика детерминированным и облегчает проверку логики предположений.
closes: #10
Ожидаем релиза: oscript-library/asserts#15
Summary by CodeRabbit
Новые возможности
Исправления
Документация
✏️ Tip: You can customize this high-level summary in your review settings.