Skip to content

Commit

Permalink
Addressing PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@avimanyum
avimanyum committed Apr 4, 2023
1 parent d20c1fa commit 79f71bd
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 20 deletions.
18 changes: 10 additions & 8 deletions ...update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,17 +277,21 @@ public GHBlob tryRetrievingBlob(GHRepository repo, String path, String branch)
return gitHubUtil.tryRetrievingBlob(repo, path, branch);
}

public boolean modifyOnGithub(GHContent content,
public void modifyOnGithub(GHContent content,
String branch, String img, String tag,
String customMessage, String ignoreImageString) throws IOException {
boolean modified;
modifyContentOnGithub(content, branch, img, tag, customMessage, ignoreImageString);
}

protected boolean modifyContentOnGithub(GHContent content,
String branch, String img, String tag,
String customMessage, String ignoreImageString) throws IOException {
try (InputStream stream = content.read();
InputStreamReader streamR = new InputStreamReader(stream);
BufferedReader reader = new BufferedReader(streamR)) {
modified = findImagesAndFix(content, branch, img, tag, customMessage, reader,
return findImagesAndFix(content, branch, img, tag, customMessage, reader,
ignoreImageString);
}
return modified;
}

protected boolean findImagesAndFix(GHContent content, String branch, String img,
Expand Down Expand Up @@ -546,12 +550,10 @@ public void changeDockerfiles(Namespace ns,
if (content == null) {
log.info("No Dockerfile found at path: '{}'", pathToDockerfile);
} else {
if (modifyOnGithub(content, gitForkBranch.getBranchName(),
isContentModified |= modifyContentOnGithub(content, gitForkBranch.getBranchName(),
gitForkBranch.getImageName(), gitForkBranch.getImageTag(),
ns.get(Constants.GIT_ADDITIONAL_COMMIT_MESSAGE),
ns.get(Constants.IGNORE_IMAGE_STRING))) {
isContentModified = true;
}
ns.get(Constants.IGNORE_IMAGE_STRING));
isRepoSkipped = false;
}
}
Expand Down
21 changes: 9 additions & 12 deletions ...te/src/test/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -831,7 +831,6 @@ public void testOnePullRequestForMultipleDockerfilesInSameRepo() throws Exceptio
when(parentRepo.getFullName()).thenReturn("repo1");
when(forkedRepo.getParent()).thenReturn(parentRepo);

//GHRepository parent = mock(GHRepository.class);
String defaultBranch = "default";
when(parentRepo.getDefaultBranch()).thenReturn(defaultBranch);
GHBranch parentBranch = mock(GHBranch.class);
Expand All @@ -854,13 +853,13 @@ public void testOnePullRequestForMultipleDockerfilesInSameRepo() throws Exceptio
eq("df12"), eq("image-tag"))).thenReturn(forkedRepoContent2);

//Both Dockerfiles modified
doReturn(true).when(dockerfileGitHubUtil).modifyOnGithub(eq(forkedRepoContent1),
doReturn(true).when(dockerfileGitHubUtil).modifyContentOnGithub(eq(forkedRepoContent1),
eq("image-tag"),
eq("image"),
eq("tag"),
eq(null),
eq(null));
doReturn(true).when(dockerfileGitHubUtil).modifyOnGithub(eq(forkedRepoContent2),
doReturn(true).when(dockerfileGitHubUtil).modifyContentOnGithub(eq(forkedRepoContent2),
eq("image-tag"),
eq("image"),
eq("tag"),
Expand All @@ -881,7 +880,7 @@ public void testOnePullRequestForMultipleDockerfilesInSameRepo() throws Exceptio

// Both Dockerfiles modified
verify(dockerfileGitHubUtil, times(2))
.modifyOnGithub(any(), eq("image-tag"), eq("image"), eq("tag"), any(), any());
.modifyContentOnGithub(any(), eq("image-tag"), eq("image"), eq("tag"), any(), any());

// Only one PR created on the repo with changes to both Dockerfiles.
verify(dockerfileGitHubUtil).createPullReq(eq(parentRepo),
Expand All @@ -907,7 +906,6 @@ public void testPullRequestBlockNotExecutedWhenDockerfileIsNotModified() throws
when(parentRepo.getFullName()).thenReturn("repo");
when(forkedRepo.getParent()).thenReturn(parentRepo);

//GHRepository parent = mock(GHRepository.class);
String defaultBranch = "default";
when(parentRepo.getDefaultBranch()).thenReturn(defaultBranch);
GHBranch parentBranch = mock(GHBranch.class);
Expand All @@ -926,7 +924,7 @@ public void testPullRequestBlockNotExecutedWhenDockerfileIsNotModified() throws
eq("df11"), eq("image-tag"))).thenReturn(forkedRepoContent);

//Dockerfile not modified
doReturn(false).when(dockerfileGitHubUtil).modifyOnGithub(eq(forkedRepoContent),
doReturn(false).when(dockerfileGitHubUtil).modifyContentOnGithub(eq(forkedRepoContent),
eq("image-tag"),
eq("image"),
eq("tag"),
Expand All @@ -944,10 +942,10 @@ public void testPullRequestBlockNotExecutedWhenDockerfileIsNotModified() throws
eq("df11"), eq("image-tag"));

// Trying to modify Dockerfile
verify(dockerfileGitHubUtil).modifyOnGithub(any(), eq("image-tag"), eq("image"), eq("tag"), any(), any());
verify(dockerfileGitHubUtil).modifyContentOnGithub(any(), eq("image-tag"), eq("image"), eq("tag"), any(), any());

// PR creation block not executed
verify(dockerfileGitHubUtil, times(0)).createPullReq(eq(parentRepo),
verify(dockerfileGitHubUtil, never()).createPullReq(eq(parentRepo),
eq("image-tag"), eq(forkedRepo), any(), eq(rateLimiter));
}

Expand All @@ -971,7 +969,6 @@ public void testPullRequestCreatedWhenAnyDockerfileIsModified() throws Exception
when(parentRepo.getFullName()).thenReturn("repo");
when(forkedRepo.getParent()).thenReturn(parentRepo);

//GHRepository parent = mock(GHRepository.class);
String defaultBranch = "default";
when(parentRepo.getDefaultBranch()).thenReturn(defaultBranch);
GHBranch parentBranch = mock(GHBranch.class);
Expand All @@ -994,15 +991,15 @@ public void testPullRequestCreatedWhenAnyDockerfileIsModified() throws Exception
eq("df12"), eq("image-tag"))).thenReturn(forkedRepoContent2);

//First dockerfile not modified
doReturn(false).when(dockerfileGitHubUtil).modifyOnGithub(eq(forkedRepoContent),
doReturn(false).when(dockerfileGitHubUtil).modifyContentOnGithub(eq(forkedRepoContent),
eq("image-tag"),
eq("image"),
eq("tag"),
eq(null),
eq(null));

//Second dockerfile modified
doReturn(true).when(dockerfileGitHubUtil).modifyOnGithub(eq(forkedRepoContent2),
doReturn(true).when(dockerfileGitHubUtil).modifyContentOnGithub(eq(forkedRepoContent2),
eq("image-tag"),
eq("image"),
eq("tag"),
Expand All @@ -1022,7 +1019,7 @@ public void testPullRequestCreatedWhenAnyDockerfileIsModified() throws Exception
eq("df12"), eq("image-tag"));

// Trying to modify both Dockerfiles
verify(dockerfileGitHubUtil, times(2)).modifyOnGithub(any(), eq("image-tag"), eq("image")
verify(dockerfileGitHubUtil, times(2)).modifyContentOnGithub(any(), eq("image-tag"), eq("image")
, eq("tag"), any(), any());

// PR created
Expand Down

0 comments on commit 79f71bd

Please sign in to comment.