Многострочные строковые литералы и исправление тестирования #11
Conversation
sfaqer
commented
Jan 30, 2025
sfaqer
commented
- Теперь тесты вновь запускаются
- Поднята зависимость от декоратора
- Теперь в описаниях команд\опций\аргументов можно использовать многострочные строковые литералы
2. Поднята зависимость от декоратора 3. Теперь в описаниях команд\опций\аргументов можно использовать многострочные строковые литералы
|
Caution Review failedThe pull request is closed. WalkthroughОбновление включает в себя несколько изменений в проекте autumn-cli. Версия пакета увеличена до 1.1.0, обновлена версия среды до 1.9.2 и зависимость от пакета autumn до версии 4.3.7. Также обновлена зависимость от пакета decorator до версии 2.0.2. В коде декоратора команд улучшена обработка многострочных описаний. Из тестового скрипта удалена функция выполнения BDD-тестов, добавлены новые тестовые классы и исправлена опечатка в имени процедуры. Также был добавлен новый скрипт для управления покрытием кода в Windows. Удален файл package-loader.os. Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
2. Отказ от кастомного загрузчика библиотеки
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
tasks/test.os (1)
39-39: Проверка статуса тестов.
Отсутствие детальной обработки ошибок BDD оставляет только общий результат тестирования. Убедитесь, что это не влияет на выявление специфических ошибок.tasks/coverage.os (2)
8-9: Используйте более надежный способ работы с путями.Текущая реализация может быть улучшена использованием встроенных методов для работы с путями.
Предлагаю следующие изменения:
-ФС.ОбеспечитьПустойКаталог(ОбъединитьПути("out", "coverage")); -ПутьКСтат = "out/coverage/stat.json"; +ПутьККаталогу = ОбъединитьПути(ТекущийКаталог(), "out", "coverage"); +ФС.ОбеспечитьПустойКаталог(ПутьККаталогу); +ПутьКСтат = ОбъединитьПути(ПутьККаталогу, "stat.json");
34-34: Добавьте обработку кодов возврата.Текущая реализация просто передает код возврата, но было бы полезно добавить логирование или обработку ошибок.
Предлагаю следующие изменения:
-ЗавершитьРаботу(КодВозврата); +Если КодВозврата <> 0 Тогда + ВызватьИсключение СтрШаблон("Выполнение тестов завершилось с ошибкой. Код возврата: %1", КодВозврата); +КонецЕсли; + +ЗавершитьРаботу(КодВозврата);src/Классы/ИКС_ДекораторКоманд.os (1)
131-133: Добавьте проверку на пустое описание.Текущая реализация не проверяет случай, когда описание может быть пустым или равным
Неопределено.Предлагаю следующие изменения:
+Если Описание = Неопределено ИЛИ ПустаяСтрока(Описание) Тогда + Возврат; +КонецЕсли; + Если СтрЧислоСтрок(Описание) > 1 Тогда Описание = СтрЗаменить(Описание, Символы.ПС, Символы.ПС + "|"); КонецЕсли;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
.github/workflows/test.ymlis excluded by!**/*.yml
📒 Files selected for processing (8)
packagedef(1 hunks)src/Классы/ИКС_ДекораторКоманд.os(1 hunks)tasks/coverage.os(1 hunks)tasks/test.os(3 hunks)tests/alltests.os(1 hunks)tests/Классы/ДубЛегасиКоманды.os(1 hunks)tests/Классы/КомандаСАргументомИФлагом.os(1 hunks)tests/Классы/КомандаСМногострочнымиОписаниями.os(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/Классы/ДубЛегасиКоманды.os
🧰 Additional context used
🪛 GitHub Actions: Тестирование
tests/alltests.os
[error] 8-498: All 20 tests failed due to initialization error in Поделка.os
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test (dev) / build (windows-latest, dev)
- GitHub Check: test (default) / build (ubuntu-latest, default)
🔇 Additional comments (11)
tasks/test.os (2)
10-10: Уточните необходимость BDD-тестов.
Удаление логики для BDD может привести к потере части покрываемых сценариев. Если это намеренное решение, убедитесь, что все важные сценарии теперь покрываются другими тестами.
23-23: Выглядит корректно.
Завершение процедуры на данном этапе не вызывает вопросов. Функция упрощена и понятна.tests/Классы/КомандаСМногострочнымиОписаниями.os (5)
1-10: Многострочные описания аргумента.
Реализация с использованием многострочных строковых литералов улучшает читаемость и наглядность аргумента "ARG". Отличная идея для расширенной документации.
11-20: Опция с многострочным описанием.
Аналогичное улучшение, позволяющее подробнее описать параметры запуска. Хорошая практика для CLI-инструментов.
23-29: Команда с многострочным описанием.
Расширенное описание упрощает понимание назначения команды. Убедитесь, что подобные описания корректно выводятся на всех платформах и в справке.
30-32: Инициализация массива результата.
Объявление массива внутри процедуры упрощает логику хранения и обратной передачи значений, что способствует лучшей модульности.
34-39: Формирование итогового результата.
Добавление аргумента и опции в массив даёт понятную структуру данных для последующей обработки. Логика выглядит целостной и расширяемой.packagedef (3)
8-8: Обновление версии пакета.
Переход на версию «1.0.7» логичен при внесении функциональных обновлений, включая поддержку многострочных описаний команд.
12-12: Версия среды 1.9.2.
Убедитесь, что в остальных компонентах проекта учтены возможные изменения и совместимость с новой версией среды.
16-16: Обновление зависимости «decorator» до 2.0.2.
Проверьте, нет ли критических изменений, влияющих на предыдущие вызовы декораторов. Рекомендуется перезапустить тесты с расширенными сценариями.tests/Классы/КомандаСАргументомИФлагом.os (1)
7-10: Корректное объявление флага установки!Флаг
ВыполнятьУстановкуПриНеобходимостиправильно определен с использованием всех необходимых атрибутов:
- Корректное значение по умолчанию
- Привязка к переменной окружения
- Согласованность с общей архитектурой команды
src/Классы/ИКС_ДекораторКоманд.os
Outdated
| ТелоМетода.Добавить( | ||
| СтрШаблон( | ||
| Шаблон, | ||
| РаботаСАннотациями.ПолучитьЗначениеПараметраАннотации(Аннотация, "Имя"), |
tasks/test.os
Outdated
| @@ -8,7 +7,7 @@ | |||
| Тестер.УстановитьФорматЛогФайла(Тестер.ФорматыЛогФайла().GenericExec); | |||
|
|
|||
| ПутьКТестам = "tests"; | |||
| ПутьКОтчетуJUnit = "out"; | |||
| ПутьКОтчетуJUnit = ОбъединитьПути("out", "testsResults"); | |||
There was a problem hiding this comment.
А вот это зачем? Оно сонару не помешает?
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tasks/test.os (1)
Line range hint
27-38: Рекомендуется улучшить обработку ошибок.Текущая реализация обработки ошибок может быть улучшена:
- Добавьте более подробную информацию о типе ошибки
- Рассмотрите возможность логирования ошибок для последующего анализа
Предлагаемые изменения:
Попытка ТестыПрошли = ПрогнатьТесты(); Исключение ТестыПрошли = Ложь; - Сообщить(СтрШаблон("Тесты через 1testrunner выполнены неудачно - |%1", ПодробноеПредставлениеОшибки(ИнформацияОбОшибке()))); + ИнфоОбОшибке = ИнформацияОбОшибке(); + ТекстОшибки = СтрШаблон( + "Тесты через 1testrunner выполнены неудачно: + |Описание: %1 + |Модуль: %2 + |Строка: %3 + |Подробно: %4", + ИнфоОбОшибке.Описание, + ИнфоОбОшибке.ИмяМодуля, + ИнфоОбОшибке.НомерСтроки, + ПодробноеПредставлениеОшибки(ИнфоОбОшибке) + ); + Сообщить(ТекстОшибки); КонецПопытки;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/Классы/ИКС_ДекораторКоманд.os(1 hunks)tasks/test.os(2 hunks)tests/alltests.os(1 hunks)tests/Классы/КомандаСМногострочнымиОписаниями.os(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/alltests.os
- tests/Классы/КомандаСМногострочнымиОписаниями.os
- src/Классы/ИКС_ДекораторКоманд.os
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: test (default) / build (ubuntu-latest, default)
- GitHub Check: test (dev) / build (windows-latest, dev)
- GitHub Check: test (dev) / build (ubuntu-latest, dev)
- GitHub Check: test (lts-dev) / build (ubuntu-latest, lts-dev)
- GitHub Check: test (lts-dev) / build (ubuntu-latest, lts-dev)
- GitHub Check: test (default) / build (ubuntu-latest, default)
🔇 Additional comments (2)
tasks/test.os (2)
23-23: Хорошая практика добавления завершающего комментария функции!Добавление завершающего комментария улучшает читаемость кода, особенно в больших функциях.
39-44: Проверьте влияние удаления BDD тестов на покрытие кода.После удаления функциональности BDD тестов (ПрогнатьФичи) и упрощения обработки ошибок, необходимо убедиться, что все критические сценарии по-прежнему покрыты оставшимися тестами.
