Skip to content

Commit 15a6327

Browse files
afalkoafalko
authored
Merge pull request #874 from avimanyum/empty_pr_fix
Skip sending PRs if no files in the repository is modified
2 parents e8b92a5 + 79f71bd commit 15a6327

File tree

2 files changed

+172
-14
lines changed

2 files changed

+172
-14
lines changed

‎dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -280,14 +280,21 @@ public GHBlob tryRetrievingBlob(GHRepository repo, String path, String branch)
280280
public void modifyOnGithub(GHContent content,
281281
String branch, String img, String tag,
282282
String customMessage, String ignoreImageString) throws IOException {
283+
modifyContentOnGithub(content, branch, img, tag, customMessage, ignoreImageString);
284+
}
285+
286+
protected boolean modifyContentOnGithub(GHContent content,
287+
String branch, String img, String tag,
288+
String customMessage, String ignoreImageString) throws IOException {
283289
try (InputStream stream = content.read();
284290
InputStreamReader streamR = new InputStreamReader(stream);
285291
BufferedReader reader = new BufferedReader(streamR)) {
286-
findImagesAndFix(content, branch, img, tag, customMessage, reader, ignoreImageString);
292+
return findImagesAndFix(content, branch, img, tag, customMessage, reader,
293+
ignoreImageString);
287294
}
288295
}
289296

290-
protected void findImagesAndFix(GHContent content, String branch, String img,
297+
protected boolean findImagesAndFix(GHContent content, String branch, String img,
291298
String tag, String customMessage, BufferedReader reader,
292299
String ignoreImageString) throws IOException {
293300
StringBuilder strB = new StringBuilder();
@@ -296,6 +303,7 @@ protected void findImagesAndFix(GHContent content, String branch, String img,
296303
content.update(strB.toString(),
297304
"Fix Docker base image in /" + content.getPath() + "\n\n" + customMessage, branch);
298305
}
306+
return modified;
299307
}
300308

301309
protected boolean rewriteDockerfile(String img, String tag,
@@ -542,9 +550,10 @@ public void changeDockerfiles(Namespace ns,
542550
if (content == null) {
543551
log.info("No Dockerfile found at path: '{}'", pathToDockerfile);
544552
} else {
545-
modifyOnGithub(content, gitForkBranch.getBranchName(), gitForkBranch.getImageName(), gitForkBranch.getImageTag(),
546-
ns.get(Constants.GIT_ADDITIONAL_COMMIT_MESSAGE), ns.get(Constants.IGNORE_IMAGE_STRING));
547-
isContentModified = true;
553+
isContentModified |= modifyContentOnGithub(content, gitForkBranch.getBranchName(),
554+
gitForkBranch.getImageName(), gitForkBranch.getImageTag(),
555+
ns.get(Constants.GIT_ADDITIONAL_COMMIT_MESSAGE),
556+
ns.get(Constants.IGNORE_IMAGE_STRING));
548557
isRepoSkipped = false;
549558
}
550559
}
@@ -567,6 +576,8 @@ public void changeDockerfiles(Namespace ns,
567576
forkedRepo,
568577
pullRequestInfo,
569578
rateLimiter);
579+
} else {
580+
log.info("No files changed in repo {}. Skipping PR creation attempt.", parentName);
570581
}
571582
}
572583

‎dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtilTest.java‎

Lines changed: 156 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -822,8 +822,6 @@ public void testOnePullRequestForMultipleDockerfilesInSameRepo() throws Exceptio
822822
Multimap<String, GitHubContentToProcess> pathToDockerfilesInParentRepo = HashMultimap.create();
823823
pathToDockerfilesInParentRepo.put("repo1", new GitHubContentToProcess(null, null, "df11"));
824824
pathToDockerfilesInParentRepo.put("repo1", new GitHubContentToProcess(null, null, "df12"));
825-
pathToDockerfilesInParentRepo.put("repo3", new GitHubContentToProcess(null, null, "df3"));
826-
pathToDockerfilesInParentRepo.put("repo4", new GitHubContentToProcess(null, null, "df4"));
827825

828826
GHRepository forkedRepo = mock(GHRepository.class);
829827
when(forkedRepo.isFork()).thenReturn(true);
@@ -833,7 +831,6 @@ public void testOnePullRequestForMultipleDockerfilesInSameRepo() throws Exceptio
833831
when(parentRepo.getFullName()).thenReturn("repo1");
834832
when(forkedRepo.getParent()).thenReturn(parentRepo);
835833

836-
//GHRepository parent = mock(GHRepository.class);
837834
String defaultBranch = "default";
838835
when(parentRepo.getDefaultBranch()).thenReturn(defaultBranch);
839836
GHBranch parentBranch = mock(GHBranch.class);
@@ -843,9 +840,6 @@ public void testOnePullRequestForMultipleDockerfilesInSameRepo() throws Exceptio
843840

844841
gitHubUtil = mock(GitHubUtil.class);
845842
dockerfileGitHubUtil = Mockito.spy(new DockerfileGitHubUtil(gitHubUtil));
846-
//when(dockerfileGitHubUtil.getPullRequestForImageBranch(any(), any())).thenReturn
847-
// (Optional.empty());
848-
//when(dockerfileGitHubUtil.getRepo(forkedRepo.getFullName())).thenReturn(forkedRepo);
849843
InputStream stream = new ByteArrayInputStream("someContent".getBytes(StandardCharsets.UTF_8));
850844
GHContent forkedRepoContent1 = mock(GHContent.class);
851845
when(forkedRepoContent1.read()).thenReturn(stream);
@@ -857,8 +851,21 @@ public void testOnePullRequestForMultipleDockerfilesInSameRepo() throws Exceptio
857851
when(forkedRepoContent2.read()).thenReturn(stream);
858852
when(gitHubUtil.tryRetrievingContent(eq(forkedRepo),
859853
eq("df12"), eq("image-tag"))).thenReturn(forkedRepoContent2);
860-
doNothing().when(dockerfileGitHubUtil).modifyOnGithub(any(), eq("image-tag"), eq("image")
861-
, eq("tag"), anyString(), anyString());
854+
855+
//Both Dockerfiles modified
856+
doReturn(true).when(dockerfileGitHubUtil).modifyContentOnGithub(eq(forkedRepoContent1),
857+
eq("image-tag"),
858+
eq("image"),
859+
eq("tag"),
860+
eq(null),
861+
eq(null));
862+
doReturn(true).when(dockerfileGitHubUtil).modifyContentOnGithub(eq(forkedRepoContent2),
863+
eq("image-tag"),
864+
eq("image"),
865+
eq("tag"),
866+
eq(null),
867+
eq(null));
868+
862869
doNothing().when(rateLimiter).consume();
863870

864871
dockerfileGitHubUtil.changeDockerfiles(ns, pathToDockerfilesInParentRepo,
@@ -873,13 +880,153 @@ public void testOnePullRequestForMultipleDockerfilesInSameRepo() throws Exceptio
873880

874881
// Both Dockerfiles modified
875882
verify(dockerfileGitHubUtil, times(2))
876-
.modifyOnGithub(any(), eq("image-tag"), eq("image"), eq("tag"), any(), any());
883+
.modifyContentOnGithub(any(), eq("image-tag"), eq("image"), eq("tag"), any(), any());
877884

878885
// Only one PR created on the repo with changes to both Dockerfiles.
879886
verify(dockerfileGitHubUtil).createPullReq(eq(parentRepo),
880887
eq("image-tag"), eq(forkedRepo), any(), eq(rateLimiter));
881888
}
882889

890+
@Test
891+
public void testPullRequestBlockNotExecutedWhenDockerfileIsNotModified() throws Exception {
892+
Map<String, Object> nsMap = ImmutableMap.of(Constants.IMG,
893+
"image", Constants.TAG,
894+
"tag", Constants.STORE,
895+
"store");
896+
Namespace ns = new Namespace(nsMap);
897+
GitForkBranch gitForkBranch = new GitForkBranch("image", "tag", null, "");
898+
Multimap<String, GitHubContentToProcess> pathToDockerfilesInParentRepo = HashMultimap.create();
899+
pathToDockerfilesInParentRepo.put("repo", new GitHubContentToProcess(null, null, "df11"));
900+
901+
GHRepository forkedRepo = mock(GHRepository.class);
902+
when(forkedRepo.isFork()).thenReturn(true);
903+
when(forkedRepo.getFullName()).thenReturn("forkedrepo");
904+
905+
GHRepository parentRepo = mock(GHRepository.class);
906+
when(parentRepo.getFullName()).thenReturn("repo");
907+
when(forkedRepo.getParent()).thenReturn(parentRepo);
908+
909+
String defaultBranch = "default";
910+
when(parentRepo.getDefaultBranch()).thenReturn(defaultBranch);
911+
GHBranch parentBranch = mock(GHBranch.class);
912+
String sha = "abcdef";
913+
when(parentBranch.getSHA1()).thenReturn(sha);
914+
when(parentRepo.getBranch(defaultBranch)).thenReturn(parentBranch);
915+
916+
gitHubUtil = mock(GitHubUtil.class);
917+
dockerfileGitHubUtil = Mockito.spy(new DockerfileGitHubUtil(gitHubUtil));
918+
InputStream stream = new ByteArrayInputStream("someContent".getBytes(StandardCharsets.UTF_8));
919+
GHContent forkedRepoContent = mock(GHContent.class);
920+
when(forkedRepoContent.read()).thenReturn(stream);
921+
922+
RateLimiter rateLimiter = mock(RateLimiter.class);
923+
when(gitHubUtil.tryRetrievingContent(eq(forkedRepo),
924+
eq("df11"), eq("image-tag"))).thenReturn(forkedRepoContent);
925+
926+
//Dockerfile not modified
927+
doReturn(false).when(dockerfileGitHubUtil).modifyContentOnGithub(eq(forkedRepoContent),
928+
eq("image-tag"),
929+
eq("image"),
930+
eq("tag"),
931+
eq(null),
932+
eq(null));
933+
934+
doNothing().when(rateLimiter).consume();
935+
936+
dockerfileGitHubUtil.changeDockerfiles(ns, pathToDockerfilesInParentRepo,
937+
new GitHubContentToProcess(forkedRepo, parentRepo, ""), new ArrayList<>(),
938+
gitForkBranch, rateLimiter);
939+
940+
// Dockerfile retrieved from the repo
941+
verify(dockerfileGitHubUtil).tryRetrievingContent(eq(forkedRepo),
942+
eq("df11"), eq("image-tag"));
943+
944+
// Trying to modify Dockerfile
945+
verify(dockerfileGitHubUtil).modifyContentOnGithub(any(), eq("image-tag"), eq("image"), eq("tag"), any(), any());
946+
947+
// PR creation block not executed
948+
verify(dockerfileGitHubUtil, never()).createPullReq(eq(parentRepo),
949+
eq("image-tag"), eq(forkedRepo), any(), eq(rateLimiter));
950+
}
951+
952+
@Test
953+
public void testPullRequestCreatedWhenAnyDockerfileIsModified() throws Exception {
954+
Map<String, Object> nsMap = ImmutableMap.of(Constants.IMG,
955+
"image", Constants.TAG,
956+
"tag", Constants.STORE,
957+
"store");
958+
Namespace ns = new Namespace(nsMap);
959+
GitForkBranch gitForkBranch = new GitForkBranch("image", "tag", null, "");
960+
Multimap<String, GitHubContentToProcess> pathToDockerfilesInParentRepo = HashMultimap.create();
961+
pathToDockerfilesInParentRepo.put("repo", new GitHubContentToProcess(null, null, "df11"));
962+
pathToDockerfilesInParentRepo.put("repo", new GitHubContentToProcess(null, null, "df12"));
963+
964+
GHRepository forkedRepo = mock(GHRepository.class);
965+
when(forkedRepo.isFork()).thenReturn(true);
966+
when(forkedRepo.getFullName()).thenReturn("forkedrepo");
967+
968+
GHRepository parentRepo = mock(GHRepository.class);
969+
when(parentRepo.getFullName()).thenReturn("repo");
970+
when(forkedRepo.getParent()).thenReturn(parentRepo);
971+
972+
String defaultBranch = "default";
973+
when(parentRepo.getDefaultBranch()).thenReturn(defaultBranch);
974+
GHBranch parentBranch = mock(GHBranch.class);
975+
String sha = "abcdef";
976+
when(parentBranch.getSHA1()).thenReturn(sha);
977+
when(parentRepo.getBranch(defaultBranch)).thenReturn(parentBranch);
978+
979+
gitHubUtil = mock(GitHubUtil.class);
980+
dockerfileGitHubUtil = Mockito.spy(new DockerfileGitHubUtil(gitHubUtil));
981+
InputStream stream = new ByteArrayInputStream("someContent".getBytes(StandardCharsets.UTF_8));
982+
GHContent forkedRepoContent = mock(GHContent.class);
983+
when(forkedRepoContent.read()).thenReturn(stream);
984+
985+
RateLimiter rateLimiter = mock(RateLimiter.class);
986+
when(gitHubUtil.tryRetrievingContent(eq(forkedRepo),
987+
eq("df11"), eq("image-tag"))).thenReturn(forkedRepoContent);
988+
GHContent forkedRepoContent2 = mock(GHContent.class);
989+
when(forkedRepoContent2.read()).thenReturn(stream);
990+
when(gitHubUtil.tryRetrievingContent(eq(forkedRepo),
991+
eq("df12"), eq("image-tag"))).thenReturn(forkedRepoContent2);
992+
993+
//First dockerfile not modified
994+
doReturn(false).when(dockerfileGitHubUtil).modifyContentOnGithub(eq(forkedRepoContent),
995+
eq("image-tag"),
996+
eq("image"),
997+
eq("tag"),
998+
eq(null),
999+
eq(null));
1000+
1001+
//Second dockerfile modified
1002+
doReturn(true).when(dockerfileGitHubUtil).modifyContentOnGithub(eq(forkedRepoContent2),
1003+
eq("image-tag"),
1004+
eq("image"),
1005+
eq("tag"),
1006+
eq(null),
1007+
eq(null));
1008+
1009+
doNothing().when(rateLimiter).consume();
1010+
1011+
dockerfileGitHubUtil.changeDockerfiles(ns, pathToDockerfilesInParentRepo,
1012+
new GitHubContentToProcess(forkedRepo, parentRepo, ""), new ArrayList<>(),
1013+
gitForkBranch, rateLimiter);
1014+
1015+
// Both Dockerfiles retrieved from the same repo
1016+
verify(dockerfileGitHubUtil).tryRetrievingContent(eq(forkedRepo),
1017+
eq("df11"), eq("image-tag"));
1018+
verify(dockerfileGitHubUtil).tryRetrievingContent(eq(forkedRepo),
1019+
eq("df12"), eq("image-tag"));
1020+
1021+
// Trying to modify both Dockerfiles
1022+
verify(dockerfileGitHubUtil, times(2)).modifyContentOnGithub(any(), eq("image-tag"), eq("image")
1023+
, eq("tag"), any(), any());
1024+
1025+
// PR created
1026+
verify(dockerfileGitHubUtil).createPullReq(eq(parentRepo),
1027+
eq("image-tag"), eq(forkedRepo), any(), eq(rateLimiter));
1028+
}
1029+
8831030
@DataProvider
8841031
public Object[][] fromInstructionWithIgnoreStringData() {
8851032
return new Object[][] {

0 commit comments

Comments
 (0)