-
Notifications
You must be signed in to change notification settings - Fork 78
Linkage Monitor to run 2-phase model building #855
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
Conversation
| RepositoryUtility.newRepositorySystem(), | ||
| new Bom( | ||
| "com.google.cloud.tools:test-bom:0.0.1", | ||
| "com.google.guava:guava-bom:27.1-android", |
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.
Because this change involves Maven's model building, we cannot use dummy artifact here.
| Version highestGuava = versionScheme.parseVersion(guavaHighestVersion); | ||
| Version guava28 = versionScheme.parseVersion("28.0"); | ||
| // The logic should work for both jar and pom artifacts | ||
| for (String artifactId : ImmutableList.of("guava", "guava-bom")) { |
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.
This test is to ensure findHighestVersion works for BOM (extension: pom) too.
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.
make that a comment in the code
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.
Added
| ArtifactResult googleCloudBomResult = | ||
| system.resolveArtifact( | ||
| session, new ArtifactRequest(googleCloudBom0_106, ImmutableList.of(CENTRAL), null)); | ||
| doReturn(googleCloudBomResult) |
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.
doReturn for spy object
https://static.javadoc.io/org.mockito/mockito-core/3.0.0/org/mockito/Mockito.html#spy-T-
(If possible, spy object should be avoided. but Maven is tightly coupled with RepositorySystem and thus normal mock object would make the test even complicated. )
| RepositorySystem repositorySystem, RepositorySystemSession session, String bomCoordinates) | ||
| throws ModelBuildingException, ArtifactResolutionException, MavenRepositoryException { | ||
|
|
||
| // BOM Coordinates may not have extension. |
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.
may --> might
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.
Fixed.
| Version highestGuava = versionScheme.parseVersion(guavaHighestVersion); | ||
| Version guava28 = versionScheme.parseVersion("28.0"); | ||
| // The logic should work for both jar and pom artifacts | ||
| for (String artifactId : ImmutableList.of("guava", "guava-bom")) { |
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.
make that a comment in the code
Fixes #853
Linkage Monitor to run 2-phase model building. It swaps BOM version between the phases.
As per https://maven.apache.org/ref/3.6.1/maven-model-builder/
This change updates the content of the BOM after Phase 1.