Публикация результатов синтаксического контроля в Allure #148
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughВ этом обновлении представлены изменения, связанные с улучшением процесса синтаксического контроля в Jenkins-библиотеке. Основные модификации включают обновление версии библиотеки jsonschema, введение новых опций для публикации отчетов, создание нового класса Changes
Assessment against linked issues
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/ru/pulsar/jenkins/library/configuration/SyntaxCheckOptions.groovy (1)
5-5: Проверка необходимости импорта JsonIncludeИмпорт
com.fasterxml.jackson.annotation.JsonIncludeдобавлен, но не используется в данном классе. Рекомендуется удалить неиспользуемый импорт для поддержания чистоты кода.src/ru/pulsar/jenkins/library/steps/SyntaxCheck.groovy (2)
35-46: Рекомендуется упростить создание путей для отчетовНаблюдается дублирование кода при создании путей для JUnit и Allure отчетов. Рекомендуется вынести общую логику в отдельный метод.
Предлагаемый рефакторинг:
+ private String setupReportPath(String basePath, String reportType) { + FilePath pathToReport = FileUtils.getFilePath("$env.WORKSPACE/$basePath") + return FileUtils.getLocalPath(pathToReport.getParent()) + } + def run() { // ... existing code ... - String junitReport = "build/out/jUnit/syntax-check/syntax-check.xml" - FilePath pathToJUnitReport = FileUtils.getFilePath("$env.WORKSPACE/$junitReport") - String junitReportDir = FileUtils.getLocalPath(pathToJUnitReport.getParent()) + String junitReportDir = setupReportPath("build/out/jUnit/syntax-check/syntax-check.xml", "junit") - String allureReport = "build/out/allure/syntax-check/allure.xml" - FilePath pathToAllureReport = FileUtils.getFilePath("$env.WORKSPACE/$allureReport") - String allureReportDir = FileUtils.getLocalPath(pathToAllureReport.getParent()) + String allureReportDir = setupReportPath("build/out/allure/syntax-check/allure.xml", "allure")
47-80: Рекомендуется улучшить построение командыТекущая реализация построения команды через конкатенацию строк может быть улучшена использованием StringBuilder или списка параметров.
Предлагаемый рефакторинг:
+ private List<String> buildCommandParameters(String vrunnerPath) { + List<String> params = [] + params.add(vrunnerPath) + params.add("syntax-check") + params.add("--ibconnection \"/F./build/ib\"") + + if (options.groupErrorsByMetadata) { + params.add("--groupbymetadata") + } + // ... остальные параметры + return params + } + def run() { // ... existing code ... - String command = "$vrunnerPath syntax-check --ibconnection \"/F./build/ib\"" - if (options.groupErrorsByMetadata) { - command += ' --groupbymetadata' - } - // ... остальные условия + List<String> commandParams = buildCommandParameters(vrunnerPath) + String command = commandParams.join(" ")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
resources/globalConfiguration.jsonis excluded by!**/*.jsonresources/schema.jsonis excluded by!**/*.json
📒 Files selected for processing (6)
build.gradle.kts(1 hunks)src/ru/pulsar/jenkins/library/configuration/SyntaxCheckOptions.groovy(2 hunks)src/ru/pulsar/jenkins/library/steps/PublishAllure.groovy(1 hunks)src/ru/pulsar/jenkins/library/steps/SyntaxCheck.groovy(1 hunks)vars/pipeline1C.groovy(1 hunks)vars/syntaxCheck.groovy(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (10)
vars/syntaxCheck.groovy (2)
3-3: Подтверждение импорта нового классаИмпорт класса
SyntaxCheckиз пакетаru.pulsar.jenkins.library.stepsвыполнен корректно и соответствует рефакторингу кода.
9-10: Рефакторинг вызова синтаксической проверкиИспользование объекта
SyntaxCheckдля выполнения синтаксической проверки улучшает модульность и читаемость кода.src/ru/pulsar/jenkins/library/configuration/SyntaxCheckOptions.groovy (2)
31-41: Добавление опций для управления публикацией отчетовВведены новые свойства
publishToAllureReportиpublishToJUnitReportс соответствующими аннотациями и значениями по умолчанию. Это позволяет гибко управлять публикацией результатов синтаксического контроля.
50-52: Обновление метода toString()Метод
toString()обновлен для отображения новых свойств, что обеспечивает полное представление объекта при логировании или отладке.src/ru/pulsar/jenkins/library/steps/PublishAllure.groovy (1)
42-44: Интеграция результатов синтаксического контроля в AllureДобавлена проверка флагов
syntaxCheckиpublishToAllureReportперед вызовомsafeUnstash. Это гарантирует, что результаты синтаксической проверки будут включены в отчет Allure только при соответствующих настройках.src/ru/pulsar/jenkins/library/steps/SyntaxCheck.groovy (3)
1-19: Структура класса реализована корректно!Класс имеет четкую структуру с правильной инициализацией через конструктор и константой для Allure отчетов.
21-33: Корректная инициализация метода run!Реализована правильная последовательность проверок и получения зависимостей. Хорошо использован паттерн раннего возврата.
50-52: Необходимо отслеживать статус issue #361Закомментированный параметр workspace связан с открытым issue. Рекомендуется добавить ссылку на задачу в системе отслеживания для последующего возврата к этому параметру.
✅ Verification successful
Параметр --workspace временно отключен корректно
Подтверждаю, что issue #361 в репозитории vanessa-runner все еще открыт и находится в работе (milestone 3.x). Текущее решение с отключением параметра --workspace и сохранением ссылки на issue в комментарии является правильным подходом.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Проверка статуса issue gh issue view 361 --repo vanessa-opensource/vanessa-runnerLength of output: 345
vars/pipeline1C.groovy (1)
251-263: Улучшена структура этапа синтаксического контроля!Разделение на отдельные этапы "Распаковка ИБ" и "Выполнение синтаксического контроля" соответствует общей структуре пайплайна и улучшает читаемость кода.
build.gradle.kts (1)
39-39: Проверить совместимость новой версии jsonschemaОбновление версии jsonschema с 4.36.0 до 4.37.0 требует проверки на наличие критических изменений.
✅ Verification successful
Обновление jsonschema до версии 4.37.0 безопасно ✅
Анализ списка изменений показывает, что обновление содержит только улучшения и исправления ошибок:
- Добавлена новая опция для обработки null-значений
- Исправлены ошибки при работе с enum и аннотациями
- Улучшена поддержка Jackson аннотаций
- Добавлены новые возможности для Maven плагина
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Проверка списка изменений между версиями curl -s "https://api.github.com/repos/victools/jsonschema-generator/releases/tags/v4.37.0" | jq -r '.body'Length of output: 1245
| // Запуск синтакс-проверки | ||
| VRunner.exec(command, true) | ||
|
|
||
| if (options.publishToAllureReport) { | ||
| steps.stash(ALLURE_STASH, "$allureReportDir/**", true) | ||
| steps.archiveArtifacts("$allureReportDir/**") | ||
| } | ||
|
|
||
| if (options.publishToJUnitReport) { | ||
| steps.junit("$junitReportDir/*.xml", true) | ||
| steps.archiveArtifacts("$junitReportDir/**") | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Рекомендуется улучшить обработку ошибок
При выполнении команды и архивации отчетов отсутствует явная обработка исключений, что может затруднить диагностику проблем.
Предлагаемый рефакторинг:
def run() {
// ... existing code ...
+ try {
VRunner.exec(command, true)
if (options.publishToAllureReport) {
steps.stash(ALLURE_STASH, "$allureReportDir/**", true)
steps.archiveArtifacts("$allureReportDir/**")
}
if (options.publishToJUnitReport) {
steps.junit("$junitReportDir/*.xml", true)
steps.archiveArtifacts("$junitReportDir/**")
}
+ } catch (Exception e) {
+ Logger.println("Ошибка при выполнении синтаксической проверки: ${e.message}")
+ throw e
+ }📝 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.
| // Запуск синтакс-проверки | |
| VRunner.exec(command, true) | |
| if (options.publishToAllureReport) { | |
| steps.stash(ALLURE_STASH, "$allureReportDir/**", true) | |
| steps.archiveArtifacts("$allureReportDir/**") | |
| } | |
| if (options.publishToJUnitReport) { | |
| steps.junit("$junitReportDir/*.xml", true) | |
| steps.archiveArtifacts("$junitReportDir/**") | |
| } | |
| } | |
| // Запуск синтакс-проверки | |
| try { | |
| VRunner.exec(command, true) | |
| if (options.publishToAllureReport) { | |
| steps.stash(ALLURE_STASH, "$allureReportDir/**", true) | |
| steps.archiveArtifacts("$allureReportDir/**") | |
| } | |
| if (options.publishToJUnitReport) { | |
| steps.junit("$junitReportDir/*.xml", true) | |
| steps.archiveArtifacts("$junitReportDir/**") | |
| } | |
| } catch (Exception e) { | |
| Logger.println("Ошибка при выполнении синтаксической проверки: ${e.message}") | |
| throw e | |
| } | |
| } |
|
@nixel2007 в описании релиза надо будет указать, что в это не breaking change, но упомянуть об этом стоит |
fixes #142
Заодно перенес код из vars в отдельный класс, + унифицировал параметры и последовательность выполнения с другими этапами.
Я решил, что для той же пресловутой унификации правильнее будет не передавать в Allure результаты jUnit , а сразу выполнять команду vrunner syntax-check с параметром
--allure-results2.