-
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
Замер покрытия #98
base: develop
Are you sure you want to change the base?
Замер покрытия #98
Conversation
closes #39 |
src/ru/pulsar/jenkins/library/configuration/ConfigurationReader.groovy
Outdated
Show resolved
Hide resolved
src/ru/pulsar/jenkins/library/configuration/SmokeTestOptions.groovy
Outdated
Show resolved
Hide resolved
List<String> logosConfig = ["LOGOS_CONFIG=$config.logosConfig"] | ||
steps.withEnv(logosConfig) { | ||
steps.installLocalDependencies() | ||
|
||
steps.createDir('build/out') | ||
|
||
def coverageOpts = config.coverageOptions; | ||
if (options.coverage) { | ||
steps.start("${coverageOpts.dbgsPath} --addr=127.0.0.1 --port=1550") |
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.
Не надо ли это в параметры какие-то? Плюс вопрос в параллельном выполнении шагов bdd и smoke с включённым кавереджом на одном агенте. Второй dbgs не сможет подняться, да? Может хотя бы свободный порт вычислять?
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.
@nixel2007 а не надо выполнять bdd и smoke параллельно на одном агенте) они друг друг будут мешать в случае, когда add будет запускать клиентов тестирования (тесты открытия форм, тесты ком. интерфейса). соответственно, разделение по портам делать не надо, кмк
а про адрес и порт отладчика - да, у кого-то dbgs может жить где-то отдельно и на другом порту, но...зачем?)
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.
Не, речь про то, что шаги рухнут, так как они не смогут запустить два dbgs на одном и том же порту. Причём не только в параллельных стейджах, но и в параллельных билдах.
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.
@nixel2007 добавил в util класс, который берет рандомный порт
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.
@nixel2007 сделал, а потом осознал, что строка подключения клиентов тестирования в задается в *.json и порт там указывается фиксированный.
надо подумать над тем, как передавать номер порта внутрь xunit и в vanessa, но в этом случае на стороне этих инструментов потребуются доработки.
поэтому предлагаю пока что использовать один порт
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.
у VA порт скорее всего можно через энв передать. все ключи конфига можно через энв передать, этот тоже по идее должно быть можно. но не знаю, как там дела в bdd-части vanessa-add. да и в tdd-части тоже.
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.
предложение - заюзать https://plugins.jenkins.io/lockable-resources/ по имени ноды + ключ dbgs. ну и порт наверное стоит сменить со стандартного 1550. или дать возможность его конфигурить на уровне либы
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.
@nixel2007 сделал через lockable-resources.
Если запускать билд на одной ноде и разнести порты dbgs для bdd и smoke, то эти шаги в одном билде будут работать параллельно. Но еще один билд на той же ноде будет ждать освобождения ресурса. В README описано.
Если не замерять покрытие, то никаких ограничений нет.
src/ru/pulsar/jenkins/library/steps/EdtToDesignerFormatTransformation.groovy
Outdated
Show resolved
Hide resolved
StringJoiner coveragePathsConstructor = new StringJoiner(",") | ||
|
||
if (config.stageFlags.bdd && config.bddOptions.coverage) { | ||
steps.unstash("bdd-coverage") |
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.
@nixel2007 перенес, заодно унифицировал константы в Bdd и SmokeTest
|
||
if (config.stageFlags.bdd && config.bddOptions.coverage) { | ||
steps.unstash("bdd-coverage") | ||
coveragePathsConstructor.add("build/out/bdd-coverage.xml") |
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.
и это
@nixel2007 вроде как все отработал, на своих проектах проверил |
Ох, до сюда бы ещё добраться... |
f1a4ea7
to
ed17c4b
Compare
WalkthroughВнесенные изменения охватывают улучшения документации по CI-процессу для платформы 1C:Enterprise 8, добавление новых методов в интерфейсы и классы, а также введение дополнительной функциональности, связанной с измерением покрытия кода. Это включает в себя новые параметры конфигурации для различных классов, улучшенное управление ресурсами, а также обновления для интеграции покрытия в тестовые процессы. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Jenkins
participant CoverageTool
participant BDD
participant SmokeTest
participant Yaxunit
User->>Jenkins: Запуск тестов
Jenkins->>BDD: Проверка параметров покрытия
BDD->>CoverageTool: Запуск измерения покрытия
CoverageTool-->>BDD: Статус покрытия
BDD->>Jenkins: Завершение тестов
Jenkins->>SmokeTest: Проверка параметров покрытия
SmokeTest->>CoverageTool: Запуск измерения покрытия
CoverageTool-->>SmokeTest: Статус покрытия
SmokeTest->>Jenkins: Завершение тестов
Jenkins->>Yaxunit: Проверка параметров покрытия
Yaxunit->>CoverageTool: Запуск измерения покрытия
CoverageTool-->>Yaxunit: Статус покрытия
Yaxunit->>Jenkins: Завершение тестов
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (1)
src/ru/pulsar/jenkins/library/steps/Start.groovy (1)
6-23
: Текущая реализация одобрена.Класс
Start
корректно обрабатывает выполнение скриптов в зависимости от платформы. Возможно, стоит рассмотреть интеграцию этой функциональности в классCmd
для упрощения архитектуры в будущем.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
resources/globalConfiguration.json
is excluded by!**/*.json
resources/schema.json
is excluded by!**/*.json
test/integration/resources/jobConfiguration.json
is excluded by!**/*.json
Files selected for processing (19)
- README.md (5 hunks)
- src/ru/pulsar/jenkins/library/IStepExecutor.groovy (2 hunks)
- src/ru/pulsar/jenkins/library/StepExecutor.groovy (2 hunks)
- src/ru/pulsar/jenkins/library/configuration/BddOptions.groovy (1 hunks)
- src/ru/pulsar/jenkins/library/configuration/ConfigurationReader.groovy (5 hunks)
- src/ru/pulsar/jenkins/library/configuration/CoverageOptions.groovy (1 hunks)
- src/ru/pulsar/jenkins/library/configuration/JobConfiguration.groovy (2 hunks)
- src/ru/pulsar/jenkins/library/configuration/SmokeTestOptions.groovy (3 hunks)
- src/ru/pulsar/jenkins/library/configuration/YaxunitOptions.groovy (2 hunks)
- src/ru/pulsar/jenkins/library/steps/Bdd.groovy (3 hunks)
- src/ru/pulsar/jenkins/library/steps/EdtToDesignerFormatTransformation.groovy (1 hunks)
- src/ru/pulsar/jenkins/library/steps/PublishAllure.groovy (1 hunks)
- src/ru/pulsar/jenkins/library/steps/SmokeTest.groovy (5 hunks)
- src/ru/pulsar/jenkins/library/steps/SonarScanner.groovy (3 hunks)
- src/ru/pulsar/jenkins/library/steps/Start.groovy (1 hunks)
- src/ru/pulsar/jenkins/library/steps/Yaxunit.groovy (5 hunks)
- test/integration/groovy/jobConfigurationTest.groovy (1 hunks)
- test/unit/groovy/ru/pulsar/jenkins/library/configuration/ConfigurationReaderTest.java (2 hunks)
- vars/start.groovy (1 hunks)
Files skipped from review due to trivial changes (2)
- src/ru/pulsar/jenkins/library/configuration/ConfigurationReader.groovy
- src/ru/pulsar/jenkins/library/steps/EdtToDesignerFormatTransformation.groovy
Additional comments not posted (39)
src/ru/pulsar/jenkins/library/configuration/BddOptions.groovy (3)
19-20
: Добавлен новый параметрcoverage
.Параметр корректно добавлен и аннотирован, что позволяет правильно управлять замером покрытия.
22-23
: Добавлен новый параметрdbgsPort
.Параметр для указания порта сервера отладки добавлен корректно и аннотирован, что обеспечивает его правильное использование.
30-32
: Обновление методаtoString()
.Метод
toString()
обновлен для включения новых параметров, что обеспечивает корректное отображение состояния экземпляра класса.src/ru/pulsar/jenkins/library/configuration/SmokeTestOptions.groovy (3)
30-31
: Добавлен параметрcoverage
.Параметр для замера покрытия добавлен корректно, что позволяет настроить измерение покрытия во время тестирования.
33-34
: Добавлен параметрdbgsPort
.Параметр для указания порта сервера отладки добавлен корректно, что обеспечивает его правильное использование при замере покрытия.
44-45
: Обновление методаtoString()
.Метод
toString()
обновлен для включения новых параметров. Предложение изменения отnixel2007
уже учтено в текущем изменении.src/ru/pulsar/jenkins/library/configuration/YaxunitOptions.groovy (3)
30-31
: Добавлен параметрcoverage
.Параметр для замера покрытия добавлен корректно, что позволяет настроить измерение покрытия во время тестирования.
33-34
: Добавлен параметрdbgsPort
.Параметр для указания порта сервера отладки добавлен корректно, что обеспечивает его правильное использование при замере покрытия.
44-45
: Обновление методаtoString()
.Метод
toString()
обновлен для включения новых параметров, что обеспечивает корректное отображение состояния экземпляра класса.src/ru/pulsar/jenkins/library/steps/PublishAllure.groovy (2)
34-34
: Улучшение поддерживаемости кода.Замена строкового литерала на константу улучшает читаемость и управляемость кода, снижая риск ошибок при написании строк.
40-40
: Консистентное использование констант.Использование констант для имен стэшей улучшает поддерживаемость и предотвращает возможные ошибки, связанные с опечатками в строках.
test/integration/groovy/jobConfigurationTest.groovy (2)
77-77
: Проверка пути к исполняемому файлу отладчика.Добавление утверждения для проверки наличия пути к исполняемому файлу отладчика в логе увеличивает покрытие тестами и помогает убедиться в корректности конфигурации.
78-78
: Проверка пути к инструменту покрытия.Утверждение для проверки пути к инструменту покрытия в логе улучшает надежность тестов, подтверждая, что инструмент покрытия корректно интегрирован в процесс.
src/ru/pulsar/jenkins/library/IStepExecutor.groovy (2)
43-43
: Добавление метода для запуска скриптов.Метод
start
расширяет функциональность интерфейса, позволяя инициировать выполнение скриптов, что может быть полезно для различных задач в Jenkins.
69-69
: Введение механизма блокировки ресурсов.Метод
lock
добавляет возможность управления конкуренцией за ресурсы, что важно для корректной работы параллельных задач в сложных пайплайнах.src/ru/pulsar/jenkins/library/steps/Bdd.groovy (6)
15-17
: Определение констант для работы с результатами тестирования и покрытия.Добавление констант
ALLURE_STASH
,COVERAGE_STASH_NAME
,COVERAGE_STASH_PATH
улучшает читаемость и поддерживаемость кода, обеспечивая стандартизацию имен и путей для сохранения артефактов.
33-37
: Инициализация переменных для конфигурации и окружения.Код корректно инициализирует переменные
options
,env
,srcDir
,workspaceDir
, используя настройки из конфигурации и окружения. Это обеспечивает гибкость и масштабируемость в управлении настройками.
44-47
: Генерация уникального ресурса для блокировки.Использование
RandomStringUtils.random
для создания уникального идентификатора ресурса, который потом условно модифицируется в зависимости от настроек покрытия, является хорошей практикой для управления параллельным доступом.
48-57
: Управление запуском инструментов покрытия.Добавленные условные блоки для управления командами покрытия (
start
,cmd
) позволяют гибко управлять процессом измерения покрытия, что улучшает функциональность класса Bdd.
74-76
: Остановка инструментов покрытия.Команда
stop
для инструмента покрытия корректно размещена внутри условного блока, что обеспечивает правильное завершение процесса измерения покрытия.
80-84
: Сохранение результатов покрытия.Условие для сохранения результатов покрытия использует ранее определенные константы, что обеспечивает консистентность и уменьшает вероятность ошибок в путях или именах файлов.
src/ru/pulsar/jenkins/library/steps/Yaxunit.groovy (6)
20-21
: Определение констант для работы с результатами покрытия.Добавление констант
COVERAGE_STASH_NAME
,COVERAGE_STASH_PATH
улучшает читаемость и поддерживаемость кода, обеспечивая стандартизацию имен и путей для сохранения результатов покрытия.
45-47
: Инициализация переменных для конфигурации и окружения.Корректная инициализация переменных
srcDir
,workspaceDir
из конфигурации и окружения поддерживает гибкость и масштабируемость управления настройками.
71-77
: Генерация уникального ресурса для блокировки.Использование
RandomStringUtils.random
для создания уникального идентификатора ресурса, который модифицируется в зависимости от настроек покрытия, является хорошей практикой для управления параллельным доступом.
78-83
: Управление запуском инструментов покрытия.Добавленные условные блоки для управления командами покрытия (
start
,cmd
) позволяют гибко управлять процессом измерения покрытия, что улучшает функциональность класса Yaxunit.
90-92
: Остановка инструментов покрытия.Команда
stop
для инструмента покрытия корректно размещена внутри условного блока, что обеспечивает правильное завершение процесса измерения покрытия.
115-117
: Сохранение результатов покрытия.Условие для сохранения результатов покрытия использует ранее определенные константы, что обеспечивает консистентность и уменьшает вероятность ошибок в путях или именах файлов.
src/ru/pulsar/jenkins/library/configuration/JobConfiguration.groovy (2)
58-61
: Добавление новой настройки для замеров покрытия.Добавление свойства
coverageOptions
с аннотациями@JsonProperty
и@JsonPropertyDescription
улучшает конфигурируемость и документированность классаJobConfiguration
, позволяя пользователям указывать настройки измерения покрытия.
95-95
: Обновление методаtoString
.Включение
coverageOptions
в вывод методаtoString
улучшает отображение состояния экземпляров класса, что способствует лучшему логированию и отладке.src/ru/pulsar/jenkins/library/steps/SonarScanner.groovy (2)
14-14
: Удаление точки с запятой у переменнойconfig
.Изменение соответствует синтаксическим предпочтениям Groovy, где точки с запятой не обязательны.
66-92
: Добавление логики для динамического построения путей отчетов о покрытии.Внесенные изменения улучшают конфигурируемость и полезность класса
SonarScanner
, особенно в средах CI/CD, где отчеты о покрытии критически важны.src/ru/pulsar/jenkins/library/steps/SmokeTest.groovy (2)
15-17
: Переименование константы и добавление новых констант для управления данными о покрытии.Изменения отражают улучшение функциональности класса
SmokeTest
, особенно в его обработке отчетов о покрытии.
Line range hint
109-144
: Расширение логики методаexecute
для включения опций покрытия.Новая переменная
coverageOpts
используется для получения конфигураций покрытия из объектаconfig
, и контрольный поток модифицирован для включения обработки покрытия.test/unit/groovy/ru/pulsar/jenkins/library/configuration/ConfigurationReaderTest.java (1)
43-45
: Добавление дополнительных утверждений в методtestCreateJobConfigurationObject
.Эти утверждения улучшают покрытие тестов для объекта
jobConfiguration
, проверяя значенияdbgsPath
иCoverage41CPath
, а также устанавливают свойстваCoverage
вfalse
дляsmokeTestOptions
иbddOptions
.Also applies to: 59-59, 65-65
src/ru/pulsar/jenkins/library/StepExecutor.groovy (2)
75-78
: Проверка метода start.Метод
start
корректно реализует вызов скрипта черезsteps.start(script)
. Это расширяет функциональность классаStepExecutor
, позволяя ему инициировать процессы, определенные предоставленным скриптом.
152-157
: Проверка метода lock.Метод
lock
корректно реализует управление блокировками ресурсов. Использованиеsteps.lock(...)
с передачей параметровlabel
,quantity
,resource
и выполнением переданного замыкания (body
) внутри блокировки улучшает возможности управления ресурсами и конкуренцией.README.md (3)
9-9
: Уточнение описания конфигурационного файла.Добавлено уточнение о предоставлении пользователю простого декларативного конфигурационного файла, что упрощает настройку и использование библиотеки.
27-28
: Уточнение требований к агентам для замеров покрытия.В документацию добавлены требования к агентам для замеров покрытия, включая необходимость установки Coverage41C, EDT и сервера отладки dbgs. Это важно для корректной настройки среды и избежания конфликтов при параллельном выполнении.
48-48
: Добавление новой функциональности замера покрытия.Документация теперь включает информацию о возможности замера покрытия при выполнении тестов, что является значимым улучшением для отслеживания качества кода.
@JsonIgnoreProperties(ignoreUnknown = true) | ||
class CoverageOptions implements Serializable { | ||
|
||
@JsonPropertyDescription('''Путь к исполняемому файлу dbgs. | ||
По умолчанию равен /opt/1cv8/current/dbgs. | ||
''') | ||
String dbgsPath | ||
|
||
@JsonPropertyDescription('''Путь к исполняемому файлу Coverage41C | ||
По умолчанию равен Coverage41C. | ||
''') | ||
String coverage41CPath | ||
|
||
@Override | ||
@NonCPS | ||
String toString() { | ||
return "coverageOptions{" + | ||
"dbgsPath=" + dbgsPath + | ||
", coverage41CPath=" + coverage41CPath + | ||
'}' | ||
} | ||
|
||
} |
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.
Структура класса и аннотации одобрены.
Класс CoverageOptions
корректно описан с четкими аннотациями для сериализации. Рекомендуется добавить валидацию путей к файлам для увеличения надежности конфигурации.
@nixel2007 сделал rebase ветки, заодно затащил замер покрытия в шаг yaxunit. У себя проверил, работает. |
"coverage": false | ||
}, | ||
"coverage": { | ||
"dbgsPath": "/opt/1cv8/current/dbgs", |
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.
несколько смущает ОС-специфичный путь по умолчанию. да еще и не типовой.
"type" : "boolean", | ||
"description" : "Выполнять замер покрытия" | ||
}, | ||
"dbgsPort" : { |
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.
не вижу параметров dbgsPort в json с глобальной конфигурацией
@@ -64,6 +66,8 @@ interface IStepExecutor { | |||
|
|||
def withEnv(List<String> strings, Closure body) | |||
|
|||
def lock(String label, int quantity, String resource, Closure<Object> objectClosure) |
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.
def lock(String label, int quantity, String resource, Closure<Object> objectClosure) | |
def lock(String label, int quantity, String resource, Closure<Object> body) |
|
||
def coverageOpts = config.coverageOptions | ||
def port = options.dbgsPort | ||
def lockableResource = RandomStringUtils.random(9, true, false) |
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.
а зачем вообще лочить ресурс, если кавередж не используется?
if (options.coverage) { | ||
steps.start("${coverageOpts.dbgsPath} --addr=127.0.0.1 --port=$port") | ||
steps.start("${coverageOpts.coverage41CPath} start -i DefAlias -u http://127.0.0.1:$port -P $workspaceDir -s $srcDir -o $COVERAGE_STASH_PATH") | ||
steps.cmd("${coverageOpts.coverage41CPath} check -i DefAlias -u http://127.0.0.1:$port") |
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.
тут нигде не надо try воткнуть, чтобы запущенные процессы погасить в случае ошибки?
|
||
steps.lock(null, 1, lockableResource) { | ||
if (options.coverage) { | ||
steps.start("${coverageOpts.dbgsPath} --addr=127.0.0.1 --port=$port") |
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.
а кто его остановит?
@@ -62,6 +63,35 @@ class SonarScanner implements Serializable { | |||
sonarCommand += " -Dsonar.externalIssuesReportPaths=build/out/edt-generic-issue.json" | |||
} | |||
|
|||
def stageFlags = config.stageFlags | |||
|
|||
if (stageFlags.bdd && config.bddOptions.coverage |
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.
кажется это большое условие можно убрать, а потом в конце проверить, что если coveragePaths не пустой, тогда плюсовать к sonarCommand
…roovy Co-authored-by: Nikita Fedkin <nixel2007@gmail.com>
ed17c4b
to
8f948c2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (1)
src/ru/pulsar/jenkins/library/steps/Bdd.groovy (1)
Line range hint
44-84
: Предложение по улучшению управления ресурсамиИспользование случайно сгенерированной строки для блокировки ресурсов может внести элемент непредсказуемости в управление ресурсами. Рассмотрите возможность использования более предсказуемого механизма, основанного на конкретных параметрах среды выполнения.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
resources/globalConfiguration.json
is excluded by!**/*.json
resources/schema.json
is excluded by!**/*.json
test/integration/resources/jobConfiguration.json
is excluded by!**/*.json
Files selected for processing (19)
- README.md (5 hunks)
- src/ru/pulsar/jenkins/library/IStepExecutor.groovy (2 hunks)
- src/ru/pulsar/jenkins/library/StepExecutor.groovy (2 hunks)
- src/ru/pulsar/jenkins/library/configuration/BddOptions.groovy (1 hunks)
- src/ru/pulsar/jenkins/library/configuration/ConfigurationReader.groovy (5 hunks)
- src/ru/pulsar/jenkins/library/configuration/CoverageOptions.groovy (1 hunks)
- src/ru/pulsar/jenkins/library/configuration/JobConfiguration.groovy (2 hunks)
- src/ru/pulsar/jenkins/library/configuration/SmokeTestOptions.groovy (3 hunks)
- src/ru/pulsar/jenkins/library/configuration/YaxunitOptions.groovy (2 hunks)
- src/ru/pulsar/jenkins/library/steps/Bdd.groovy (3 hunks)
- src/ru/pulsar/jenkins/library/steps/EdtToDesignerFormatTransformation.groovy (1 hunks)
- src/ru/pulsar/jenkins/library/steps/PublishAllure.groovy (1 hunks)
- src/ru/pulsar/jenkins/library/steps/SmokeTest.groovy (5 hunks)
- src/ru/pulsar/jenkins/library/steps/SonarScanner.groovy (3 hunks)
- src/ru/pulsar/jenkins/library/steps/Start.groovy (1 hunks)
- src/ru/pulsar/jenkins/library/steps/Yaxunit.groovy (5 hunks)
- test/integration/groovy/jobConfigurationTest.groovy (1 hunks)
- test/unit/groovy/ru/pulsar/jenkins/library/configuration/ConfigurationReaderTest.java (2 hunks)
- vars/start.groovy (1 hunks)
Files skipped from review due to trivial changes (2)
- src/ru/pulsar/jenkins/library/configuration/ConfigurationReader.groovy
- src/ru/pulsar/jenkins/library/steps/EdtToDesignerFormatTransformation.groovy
Additional comments not posted (30)
vars/start.groovy (1)
4-9
: Основная функциональность одобрена.Функция
call
корректно регистрирует контекст и запускает скрипт. Однако, рекомендуется добавить обработку ошибок и логирование для улучшения отладки и надежности.src/ru/pulsar/jenkins/library/steps/Start.groovy (1)
6-24
: Реализация классаStart
одобрена.Класс корректно обрабатывает выполнение скриптов в зависимости от операционной системы. Рассмотрите возможность интеграции функциональности с классом
Cmd
для упрощения архитектуры в будущем.src/ru/pulsar/jenkins/library/configuration/CoverageOptions.groovy (1)
7-29
: Структура класса и аннотации одобрены.Класс
CoverageOptions
корректно описан с четкими аннотациями для сериализации. Рекомендуется добавить валидацию путей к файлам для увеличения надежности конфигурации.src/ru/pulsar/jenkins/library/configuration/BddOptions.groovy (3)
19-21
: Добавление свойстваcoverage
.Свойство корректно реализовано и хорошо документировано. Изменения одобрены.
22-24
: Добавление свойстваdbgsPort
.Свойство корректно реализовано и хорошо документировано. Изменения одобрены.
30-32
: Изменения в методеtoString
.Метод корректно обновлён для включения новых свойств, что улучшает отладку и логирование. Изменения одобрены.
src/ru/pulsar/jenkins/library/configuration/SmokeTestOptions.groovy (3)
30-32
: Добавление свойстваcoverage
.Свойство корректно реализовано и хорошо документировано. Изменения одобрены.
33-35
: Добавление свойстваdbgsPort
.Свойство корректно реализовано и хорошо документировано. Изменения одобрены.
44-45
: Изменения в методеtoString
.Метод корректно обновлён для включения новых свойств, что улучшает отладку и логирование. Изменения одобрены.
src/ru/pulsar/jenkins/library/configuration/YaxunitOptions.groovy (3)
30-32
: Добавление свойстваcoverage
.Свойство корректно реализовано и хорошо документировано. Изменения одобрены.
33-35
: Добавление свойстваdbgsPort
.Свойство корректно реализовано и хорошо документировано. Изменения одобрены.
44-45
: Изменения в методеtoString
.Метод корректно обновлён для включения новых свойств, что улучшает отладку и логирование. Изменения одобрены.
src/ru/pulsar/jenkins/library/steps/PublishAllure.groovy (2)
40-40
: Использование константы для идентификатора кэша.Замена
SmokeTest.SMOKE_ALLURE_STASH
наSmokeTest.ALLURE_STASH
улучшает поддержку и читаемость кода. Убедитесь, что константаSmokeTest.ALLURE_STASH
корректно определена и доступна в этом контексте.Verification successful
Константа SmokeTest.ALLURE_STASH корректно определена.
Константа
SmokeTest.ALLURE_STASH
определена в файлеSmokeTest.groovy
какpublic static final String ALLURE_STASH = 'smoke-allure'
. Изменение вPublishAllure.groovy
является правильным и соответствует определению.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Описание: Проверка определения константы SmokeTest.ALLURE_STASH. # Тест: Поиск определения константы. Ожидаемое: Наличие определения константы. rg --type groovy -A 5 $'SmokeTest.ALLURE_STASH'Length of output: 565
Script:
#!/bin/bash # Broader search for the definition of ALLURE_STASH in the repository. rg --type groovy -A 5 'ALLURE_STASH'Length of output: 4530
34-34
: Использование константы вместо жёстко закодированной строки.Замена
'bdd-allure'
наBdd.ALLURE_STASH
улучшает поддержку и читаемость кода. Убедитесь, что константаBdd.ALLURE_STASH
корректно определена и доступна в этом контексте.Verification successful
Константа Bdd.ALLURE_STASH корректно определена.
Константа
Bdd.ALLURE_STASH
определена в классеBdd
в файлеsrc/ru/pulsar/jenkins/library/steps/Bdd.groovy
какpublic static final String ALLURE_STASH = 'bdd-allure'
. Замена жёстко закодированной строки на константу улучшает поддержку и читаемость кода. Изменение является корректным и полезным.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Описание: Проверка определения константы Bdd.ALLURE_STASH. # Тест: Поиск определения константы. Ожидаемое: Наличие определения константы. rg --type groovy -A 5 $'Bdd.ALLURE_STASH'Length of output: 674
Script:
#!/bin/bash # Описание: Расширенный поиск определения константы Bdd.ALLURE_STASH. # Тест: Поиск определения класса или объекта Bdd, чтобы найти ALLURE_STASH. rg --type groovy -A 10 $'class Bdd' rg --type groovy -A 10 $'object Bdd'Length of output: 2055
src/ru/pulsar/jenkins/library/steps/Bdd.groovy (2)
15-17
: Одобрение изменений в константахИспользование констант вместо жестко закодированных строк улучшает поддерживаемость и читаемость кода.
80-85
: Одобрение условной логики архивацииУсловная архивация данных покрытия корректно реализована и соответствует принципу выполнения действий только при необходимости.
src/ru/pulsar/jenkins/library/steps/Yaxunit.groovy (1)
20-21
: Одобрение изменений в константахИспользование констант вместо жестко закодированных строк улучшает поддерживаемость и читаемость кода.
src/ru/pulsar/jenkins/library/configuration/JobConfiguration.groovy (1)
58-61
: Одобрение добавления свойстваcoverageOptions
Добавление свойства
coverageOptions
расширяет функциональность класса, позволяя настраивать параметры замеров покрытия. Отлично документировано.src/ru/pulsar/jenkins/library/steps/SonarScanner.groovy (3)
9-9
: Добавлен новый импорт.Добавление
StringJoiner
изru.pulsar.jenkins.library.utils
подразумевает использование этого класса для управления строками, что видно из последующих изменений в коде.
30-30
: Инициализация переменной окружения.Переменная
env
используется для доступа к переменным окружения, что является стандартной практикой в Jenkins pipelines.
68-93
: Логика обработки путей покрытия.Добавленная логика проверяет флаги этапов и настройки покрытия для различных тестов (BDD, дымовые тесты, Yaxunit). Если покрытие включено, используется
StringJoiner
для создания строки с путями к отчетам о покрытии, которая затем добавляется к команде SonarQube.+if (stageFlags.bdd && config.bddOptions.coverage + || stageFlags.smoke && config.smokeTestOptions.coverage + || stageFlags.yaxunit && config.yaxunitOptions.coverage) { + StringJoiner coveragePathsConstructor = new StringJoiner(",") + if (stageFlags.bdd && config.bddOptions.coverage) { + steps.unstash(Bdd.COVERAGE_STASH_NAME) + coveragePathsConstructor.add(Bdd.COVERAGE_STASH_PATH) + } + if (stageFlags.smoke && config.smokeTestOptions.coverage) { + steps.unstash(SmokeTest.COVERAGE_STASH_NAME) + coveragePathsConstructor.add(SmokeTest.COVERAGE_STASH_PATH) + } + if (stageFlags.yaxunit && config.yaxunitOptions.coverage) { + steps.unstash(Yaxunit.COVERAGE_STASH_NAME) + coveragePathsConstructor.add(Yaxunit.COVERAGE_STASH_PATH) + } + String coveragePaths = coveragePathsConstructor.toString() + sonarCommand += " -Dsonar.coverageReportPaths=${coveragePaths}" +}Это изменение улучшает интеграцию отчетов о покрытии в процесс анализа SonarQube, что соответствует целям pull request.
Verification successful
Логика обработки путей покрытия подтверждена.
Логика обработки путей покрытия, добавленная в
SonarScanner.groovy
, проверяет флаги этапов и настройки покрытия для различных тестов и корректно интегрируется в команду SonarQube. Эта логика используется только в данном файле, что подтверждает локализованность изменений.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Описание: Проверка использования новой логики путей покрытия. # Тест: Поиск использования новой логики в других файлах. Ожидаемый результат: только в этом файле. rg --type groovy -A 5 $'coveragePathsConstructor'Length of output: 2219
src/ru/pulsar/jenkins/library/steps/SmokeTest.groovy (2)
15-17
: Переименование и добавление констант.Переименование
SMOKE_ALLURE_STASH
вALLURE_STASH
и добавлениеCOVERAGE_STASH_NAME
иCOVERAGE_STASH_PATH
улучшает читаемость и структурирование данных о покрытии. Эти изменения поддерживают новую функциональность отчетов о покрытии.-public static final String SMOKE_ALLURE_STASH = 'smoke-allure' +public static final String ALLURE_STASH = 'smoke-allure' +public static final String COVERAGE_STASH_NAME = 'smoke-coverage' +public static final String COVERAGE_STASH_PATH = 'build/out/smoke-coverage.xml'
Line range hint
109-144
: Реализация логики покрытия.Добавлены новые переменные и условия для управления процессом покрытия в тестах. Это включает блокировку ресурсов, запуск и остановку процессов покрытия, а также условное сохранение данных о покрытии.
+def coverageOpts = config.coverageOptions +def port = options.dbgsPort +def lockableResource = RandomStringUtils.random(9, true, false) +if (options.coverage) { + lockableResource = "${env.NODE_NAME}_$port" +} +steps.lock(null, 1, lockableResource) { + if (options.coverage) { + steps.start("${coverageOpts.dbgsPath} --addr=127.0.0.1 --port=$port") + steps.start("${coverageOpts.coverage41CPath} start -i DefAlias -u http://127.0.0.1:$port -P $workspaceDir -s $srcDir -o $COVERAGE_STASH_PATH") + steps.cmd("${coverageOpts.coverage41CPath} check -i DefAlias -u http://127.0.0.1:$port") + } + steps.withEnv(logosConfig) { + VRunner.exec(command, true) + } + if (options.coverage) { + steps.cmd("${coverageOpts.coverage41CPath} stop -i DefAlias -u http://127.0.0.1:$port") + } +} +if (options.coverage) { + steps.stash(COVERAGE_STASH_NAME, COVERAGE_STASH_PATH, true) +}Эти изменения улучшают управление покрытием и его интеграцию в процесс тестирования.
test/unit/groovy/ru/pulsar/jenkins/library/configuration/ConfigurationReaderTest.java (1)
43-45
: Улучшение тестового покрытия.Добавлены новые утверждения для проверки путей и настроек покрытия в
jobConfiguration
. Это улучшает проверку конфигурации и соответствует изменениям в коде, связанным с покрытием.+assertThat(jobConfiguration.getCoverageOptions().getDbgsPath()).isEqualTo("/opt/1cv8/current/dbgs"); +assertThat(jobConfiguration.getCoverageOptions().getCoverage41CPath()).isEqualTo("Coverage41C"); +assertThat(jobConfiguration.getSmokeTestOptions().getCoverage()).isFalse(); +assertThat(jobConfiguration.getBddOptions().getCoverage()).isFalse();Эти изменения повышают надежность тестов за счет проверки новых аспектов конфигурации.
Also applies to: 59-59, 65-65
src/ru/pulsar/jenkins/library/StepExecutor.groovy (2)
75-78
: Добавлен метод startМетод
start
корректно делегирует выполнение скрипта. Это упрощает интеграцию и использование в различных частях Jenkins pipeline.
152-157
: Добавлен метод lockМетод
lock
использует замыкания для управления ресурсами, что является типичным подходом в Groovy и Jenkins. Однако, стоит убедиться, что использование замыканий и параметров соответствует лучшим практикам безопасности и управления ресурсами.Verification successful
Метод
lock
используется корректноИспользование метода
lock
в файлахBdd.groovy
,SmokeTest.groovy
иYaxunit.groovy
соответствует лучшим практикам управления ресурсами и безопасности. Замыкания выполняют команды в безопасном контексте, управляя переменными окружения и проверяя параметры покрытия.
Bdd.groovy
: Управление ресурсами и выполнение команд с проверкой покрытия.SmokeTest.groovy
: Использованиеsteps.withEnv
для управления переменными окружения.Yaxunit.groovy
: Аналогично, управление ресурсами и выполнение команд в безопасном контексте.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Описание: Проверка использования метода lock в других частях кода. # Тест: Поиск использования метода lock. Ожидаем: Корректное использование замыканий. rg --type groovy -A 5 $'lock'Length of output: 6495
Script:
#!/bin/bash # Extract detailed information about the closures used with the lock method in the identified files. # Search for the lock method usage and extract more lines to understand the closure content. rg --type groovy -A 10 'steps.lock' src/ru/pulsar/jenkins/library/steps/Yaxunit.groovy src/ru/pulsar/jenkins/library/steps/SmokeTest.groovy src/ru/pulsar/jenkins/library/steps/Bdd.groovyLength of output: 3762
README.md (2)
27-28
: Уточнение требований к агентамДобавлены новые требования к агентам для замеров покрытия, что улучшает понимание необходимой конфигурации и инструментов. Особенно важно указание на необходимость установки Coverage41C и сервера отладки dbgs.
48-48
: Упоминание о замере покрытия при выполнении тестовДобавление этого пункта расширяет функциональность и подчеркивает возможности библиотеки в контексте измерения покрытия кода, что является значительным улучшением документации.
src/ru/pulsar/jenkins/library/IStepExecutor.groovy (2)
43-44
: Добавление метода startМетод
start
корректно расширяет функциональность интерфейса, позволяя выполнять скрипты в контексте исполнителя шагов. Это важное дополнение для управления процессами.
69-70
: Добавление метода lockМетод
lock
корректно добавлен для управления доступом к общим ресурсам. Параметры метода позволяют контролировать доступ к ресурсам с помощью метки, количества и ресурса, что является важным для сложных рабочих процессов. Предыдущий комментарий отnixel2007
указывает на возможное обсуждение сигнатуры метода, рекомендуется учесть это при дальнейшей разработке.
def coverageOpts = config.coverageOptions | ||
def port = options.dbgsPort | ||
def lockableResource = RandomStringUtils.random(9, true, false) | ||
if (options.coverage) { | ||
lockableResource = "${env.NODE_NAME}_$port" | ||
} | ||
|
||
steps.lock(null, 1, lockableResource) { | ||
if (options.coverage) { | ||
steps.start("${coverageOpts.dbgsPath} --addr=127.0.0.1 --port=$port") | ||
steps.start("${coverageOpts.coverage41CPath} start -i DefAlias -u http://127.0.0.1:$port -P $workspaceDir -s $srcDir -o $COVERAGE_STASH_PATH") | ||
steps.cmd("${coverageOpts.coverage41CPath} check -i DefAlias -u http://127.0.0.1:$port") | ||
} | ||
|
||
// Выполняем команды | ||
steps.withEnv(logosConfig) { | ||
VRunner.exec(runTestsCommand, true) | ||
} | ||
|
||
if (options.coverage) { | ||
steps.cmd("${coverageOpts.coverage41CPath} stop -i DefAlias -u http://127.0.0.1:$port") | ||
} |
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.
Предложение по улучшению управления ресурсами
Использование случайно сгенерированной строки для блокировки ресурсов может внести элемент непредсказуемости в управление ресурсами. Рассмотрите возможность использования более предсказуемого механизма, основанного на конкретных параметрах среды выполнения.
rule.assertLogContains("dbgsPath=C:\\Program files\\1cv8\\8.3.12.1500\\bin\\dbgs.exe", run) | ||
rule.assertLogContains("coverage41CPath=C:\\coverage\\Coverage41C.exe", run) |
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.
Проверка путей в логах
Добавленные утверждения корректно проверяют наличие путей в логах, что является важной проверкой для конфигурации инструментов отладки и покрытия. Однако, рекомендуется использовать переменные окружения или настройки конфигурации для указания путей, чтобы обеспечить гибкость и упростить будущие обновления.
No description provided.