-
Notifications
You must be signed in to change notification settings - Fork 39
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
Improve integration test setup #1427
Conversation
/integration-test |
Looks good. No mutations were possible for these changes. |
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.
Some context.
"customManagers": [ | ||
{ | ||
"customType": "regex", | ||
"fileMatch": [ | ||
"^integration-tests/.*(-init\\.patch|\\.sh)$" | ||
], | ||
"matchStrings": [ | ||
"\\b(?<packageName>[a-z0-9_.-]+?:[a-z0-9_.-]+?):(?<currentValue>[^:]+?):[a-zA-Z0-9_-]+\\b", | ||
"<version>(?<currentValue>.*?)<!-- Renovate: (?<packageName>.*?) --></version>" | ||
], | ||
"datasourceTemplate": "maven" | ||
} | ||
], |
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.
Tested using out internal Renovate testing setup.
@@ -7,7 +7,7 @@ | |||
+ <dependency> | |||
+ <groupId>org.assertj</groupId> | |||
+ <artifactId>assertj-core</artifactId> | |||
+ <version>${assertj.version}</version> | |||
+ <version>3.26.3<!-- Renovate: org.assertj:assertj-bom --></version> |
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.
I opted for this syntax, as it's a single-line way (thus compatible with Renovate's regex manager) to link versions to their dependencies, without splitting this logic across files.
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.
One consideration :).
Nice improvement!
@@ -33,4 +32,4 @@ fi | |||
"${patch_error_prone_flags}" \ | |||
"${validation_error_prone_flags}" \ | |||
"${validation_build_flags}" \ | |||
$@ | |||
${@} |
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.
${@} | |
"${@}" |
My IntelliJ suggests this, and it seemed to work :).
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.
That works currently because we have only one additional flag to pass along (--sync
), but is not what is intended: the idea is that remaining arguments are passed along individually.
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.
Clear, lets merge!
Summary of changes: - Configure Renovate to update AssertJ and fmt-maven-plugin version references in patch files and shell scripts any time those dependencies are updated in the top-level `pom.xml`. - Enhance the `--sync` flag such that it also updates the initial patch file, avoiding drift due to line shifts.
6b87a1b
to
148acd4
Compare
Quality Gate passedIssues Measures |
Looks good. No mutations were possible for these changes. |
Suggested commit message: