-
Notifications
You must be signed in to change notification settings - Fork 70
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
Доработано падение шага при отказе скрипта с вызовом ring (EDT) #113
Conversation
Обработка ошибки EDT: утилита ring не повышает в командной строке errorlevel, поэтому при ошибке в ринге шаг не считается проваленным. Изменил метод cmd для возможности проверить текст отработки ring и дальнейшего выявления из текста ошибки
Обзор измененийWalkthrough:Обновления включают добавление нового свойства Изменения
Оценка по связанным задачам
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (10)
- src/ru/pulsar/jenkins/library/IStepExecutor.groovy (2 hunks)
- src/ru/pulsar/jenkins/library/StepExecutor.groovy (2 hunks)
- src/ru/pulsar/jenkins/library/steps/Cmd.groovy (1 hunks)
- src/ru/pulsar/jenkins/library/steps/DesignerToEdtFormatTransformation.groovy (1 hunks)
- src/ru/pulsar/jenkins/library/steps/EdtToDesignerFormatTransformation.groovy (1 hunks)
- src/ru/pulsar/jenkins/library/steps/EdtValidate.groovy (1 hunks)
- src/ru/pulsar/jenkins/library/steps/RingCommand.groovy (1 hunks)
- test/unit/groovy/ru/pulsar/jenkins/library/steps/CmdTest.java (4 hunks)
- vars/cmd.groovy (1 hunks)
- vars/ringCommand.groovy (1 hunks)
Additional comments: 10
vars/ringCommand.groovy (1)
- 4-8: В этом скрипте Groovy используется класс
RingCommand
для выполнения командыring
. Важно убедиться, что классRingCommand
корректно обрабатывает все возможные сценарии выполнения команды, включая ошибки и исключения, чтобы обеспечить надежность работы Jenkins pipeline.vars/cmd.groovy (1)
- 4-7: Добавление параметра
returnStdout
в функциюcall
позволяет контролировать, будет ли возвращен стандартный вывод команды. Это улучшение может быть полезно для детальной обработки результатов выполнения команды. Однако, стоит убедиться, что все вызовы этой функции в кодовой базе были обновлены с учетом нового параметра, чтобы избежать ошибок во время выполнения.src/ru/pulsar/jenkins/library/steps/RingCommand.groovy (1)
- 14-21: Метод
run
классаRingCommand
проверяет стандартный вывод команды на наличие строки "error" и в случае обнаружения вызывает методerror
с сообщением об ошибке. Это решение работает, если сообщения об ошибках всегда содержат слово "error". Однако, может быть полезно рассмотреть более надежные способы определения ошибок, например, через коды возврата команды или анализ более специфичных сообщений об ошибках.src/ru/pulsar/jenkins/library/steps/Cmd.groovy (1)
- 24-26: Добавление проверки на одновременное использование
returnStatus
иreturnStdout
в методеrun
классаCmd
предотвращает потенциальную путаницу в поведении метода. Однако, сообщение об ошибке "returnStatus and returnStdout are not supported at the same time" может быть не совсем понятно для пользователя без контекста. Рекомендуется добавить более подробное описание проблемы и возможные решения в сообщение об ошибке.src/ru/pulsar/jenkins/library/steps/DesignerToEdtFormatTransformation.groovy (1)
- 50-50: Замена метода
cmd
наringCommand
в классеDesignerToEdtFormatTransformation
направлена на улучшение обработки ошибок при выполнении командыring
. Это изменение увеличивает надежность обработки команд и помогает избежать проблем, связанных с неправильным определением статуса выполнения команды. Важно убедиться, что новый методringCommand
тщательно протестирован и корректно обрабатывает все возможные сценарии выполнения команды.src/ru/pulsar/jenkins/library/steps/EdtToDesignerFormatTransformation.groovy (1)
- 53-53: Использование метода
ringCommand
вместоcmd
в классеEdtToDesignerFormatTransformation
улучшает обработку ошибок при выполнении командыring
. Это изменение способствует более точному контролю за процессом преобразования формата и повышает надежность работы. Рекомендуется провести тестирование, чтобы убедиться в корректности работы новой логики во всех сценариях использования.src/ru/pulsar/jenkins/library/steps/EdtValidate.groovy (1)
- 58-58: Замена вызова метода
cmd
наringCommand
в классеEdtValidate
направлена на улучшение обработки ошибок при выполнении командыring
. Это изменение позволяет более точно определять и обрабатывать ошибки, что важно для процесса валидации. Рекомендуется провести дополнительное тестирование, чтобы убедиться в корректности обработки ошибок в новой реализации.src/ru/pulsar/jenkins/library/IStepExecutor.groovy (1)
- 30-42: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [15-39]
Добавление дополнительных параметров
returnStatus
иreturnStdout
к методамsh
,bat
, иcmd
, а также введение нового методаringCommand
в интерфейсIStepExecutor
расширяет возможности управления выполнением команд и обработки их результатов. Эти изменения позволяют более гибко настраивать поведение команд и обеспечивают лучшую поддержку обработки ошибок. Важно, чтобы все реализации этого интерфейса были обновлены с учетом новых параметров и метода, чтобы обеспечить совместимость.test/unit/groovy/ru/pulsar/jenkins/library/steps/CmdTest.java (1)
- 29-45: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [32-86]
Обновление тестов в классе
CmdTest
с добавлением дополнительных параметровanyBoolean()
к методамbat
иsh
отражает изменения в логике классаCmd
, связанные с добавлением параметровreturnStatus
иreturnStdout
. Эти изменения обеспечивают корректное тестирование новой функциональности. Важно убедиться, что все сценарии использования новых параметров полностью покрыты тестами, чтобы гарантировать их надежную работу.src/ru/pulsar/jenkins/library/StepExecutor.groovy (1)
- 23-35: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [26-67]
Обновления в классе
StepExecutor
, включая добавление параметровreturnStatus
иreturnStdout
к методамsh
,bat
, иcmd
, а также введение нового методаringCommand
, расширяют возможности класса в части выполнения команд и обработки их результатов. Эти изменения улучшают гибкость и надежность обработки команд в Jenkins pipeline. Важно провести тщательное тестирование всех изменений, чтобы убедиться в их корректной работе в различных сценариях.
src/ru/pulsar/jenkins/library/steps/DesignerToEdtFormatTransformation.groovy
Outdated
Show resolved
Hide resolved
@@ -55,7 +55,7 @@ class EdtValidate implements Serializable { | |||
def ringOpts = [Constants.DEFAULT_RING_OPTS] | |||
steps.withEnv(ringOpts) { | |||
steps.catchError { |
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.
Обрати внимание, тут catchError. Он подавит исключение от падения едт
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.
Обрати внимание, тут catchError. Он подавит исключение от падения едт
С этим непонятно, это шаг именно валидации, возможно при его падении не должен падать пайплайн и поэтому тут catchError? Ты не помнишь какую логику сюда закладывал?
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.
Падение самой валидации не должно валить пайплайн. Но если рухнул сам ринг с еггогом, то пайплайн надо таки завалить кмк, так как это сильно зааффектит анализ в сонаре потом
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (5)
- src/ru/pulsar/jenkins/library/steps/Cmd.groovy (1 hunks)
- src/ru/pulsar/jenkins/library/steps/DesignerToEdtFormatTransformation.groovy (2 hunks)
- src/ru/pulsar/jenkins/library/steps/EdtToDesignerFormatTransformation.groovy (2 hunks)
- src/ru/pulsar/jenkins/library/steps/EdtValidate.groovy (2 hunks)
- src/ru/pulsar/jenkins/library/steps/RingCommand.groovy (1 hunks)
Files skipped from review as they are similar to previous changes (5)
- src/ru/pulsar/jenkins/library/steps/Cmd.groovy
- src/ru/pulsar/jenkins/library/steps/DesignerToEdtFormatTransformation.groovy
- src/ru/pulsar/jenkins/library/steps/EdtToDesignerFormatTransformation.groovy
- src/ru/pulsar/jenkins/library/steps/EdtValidate.groovy
- src/ru/pulsar/jenkins/library/steps/RingCommand.groovy
@ivanmolodec давай я пока замержу, чтобы не протухло. а catchError на edValidate отдельно доделать можно будет. |
спасибо! |
Closes #112
Summary by CodeRabbit
Новые функции
returnStdout
в конструктор классаCmd
для возвращения стандартного вывода.ringCommand
для выполнения ring команд.Изменения
cmd
наringCommand
для изменения логики выполнения команд.