Skip to content

Commit

Permalink
Merge pull request #874 from avimanyum/empty_pr_fix
Browse files Browse the repository at this point in the history 
Skip sending PRs if no files in the repository is modified
  • Loading branch information
@afalko
afalko authored Apr 4, 2023
2 parents e8b92a5 + 79f71bd commit 15a6327
Show file tree
Hide file tree
Showing 2 changed files with 172 additions and 14 deletions.
21 changes: 16 additions & 5 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 @@ -280,14 +280,21 @@ public GHBlob tryRetrievingBlob(GHRepository repo, String path, String branch)
public void modifyOnGithub(GHContent content,
String branch, String img, String tag,
String customMessage, String ignoreImageString) throws IOException {
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)) {
findImagesAndFix(content, branch, img, tag, customMessage, reader, ignoreImageString);
return findImagesAndFix(content, branch, img, tag, customMessage, reader,
ignoreImageString);
}
}

protected void findImagesAndFix(GHContent content, String branch, String img,
protected boolean findImagesAndFix(GHContent content, String branch, String img,
String tag, String customMessage, BufferedReader reader,
String ignoreImageString) throws IOException {
StringBuilder strB = new StringBuilder();
Expand All @@ -296,6 +303,7 @@ protected void findImagesAndFix(GHContent content, String branch, String img,
content.update(strB.toString(),
"Fix Docker base image in /" + content.getPath() + "\n\n" + customMessage, branch);
}
return modified;
}

protected boolean rewriteDockerfile(String img, String tag,
Expand Down Expand Up @@ -542,9 +550,10 @@ public void changeDockerfiles(Namespace ns,
if (content == null) {
log.info("No Dockerfile found at path: '{}'", pathToDockerfile);
} else {
modifyOnGithub(content, gitForkBranch.getBranchName(), gitForkBranch.getImageName(), gitForkBranch.getImageTag(),
ns.get(Constants.GIT_ADDITIONAL_COMMIT_MESSAGE), ns.get(Constants.IGNORE_IMAGE_STRING));
isContentModified = true;
isContentModified |= modifyContentOnGithub(content, gitForkBranch.getBranchName(),
gitForkBranch.getImageName(), gitForkBranch.getImageTag(),
ns.get(Constants.GIT_ADDITIONAL_COMMIT_MESSAGE),
ns.get(Constants.IGNORE_IMAGE_STRING));
isRepoSkipped = false;
}
}
Expand All @@ -567,6 +576,8 @@ public void changeDockerfiles(Namespace ns,
forkedRepo,
pullRequestInfo,
rateLimiter);
} else {
log.info("No files changed in repo {}. Skipping PR creation attempt.", parentName);
}
}

Expand Down
165 changes: 156 additions & 9 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 @@ -822,8 +822,6 @@ public void testOnePullRequestForMultipleDockerfilesInSameRepo() throws Exceptio
Multimap<String, GitHubContentToProcess> pathToDockerfilesInParentRepo = HashMultimap.create();
pathToDockerfilesInParentRepo.put("repo1", new GitHubContentToProcess(null, null, "df11"));
pathToDockerfilesInParentRepo.put("repo1", new GitHubContentToProcess(null, null, "df12"));
pathToDockerfilesInParentRepo.put("repo3", new GitHubContentToProcess(null, null, "df3"));
pathToDockerfilesInParentRepo.put("repo4", new GitHubContentToProcess(null, null, "df4"));

GHRepository forkedRepo = mock(GHRepository.class);
when(forkedRepo.isFork()).thenReturn(true);
Expand All @@ -833,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 @@ -843,9 +840,6 @@ public void testOnePullRequestForMultipleDockerfilesInSameRepo() throws Exceptio

gitHubUtil = mock(GitHubUtil.class);
dockerfileGitHubUtil = Mockito.spy(new DockerfileGitHubUtil(gitHubUtil));
//when(dockerfileGitHubUtil.getPullRequestForImageBranch(any(), any())).thenReturn
// (Optional.empty());
//when(dockerfileGitHubUtil.getRepo(forkedRepo.getFullName())).thenReturn(forkedRepo);
InputStream stream = new ByteArrayInputStream("someContent".getBytes(StandardCharsets.UTF_8));
GHContent forkedRepoContent1 = mock(GHContent.class);
when(forkedRepoContent1.read()).thenReturn(stream);
Expand All @@ -857,8 +851,21 @@ public void testOnePullRequestForMultipleDockerfilesInSameRepo() throws Exceptio
when(forkedRepoContent2.read()).thenReturn(stream);
when(gitHubUtil.tryRetrievingContent(eq(forkedRepo),
eq("df12"), eq("image-tag"))).thenReturn(forkedRepoContent2);
doNothing().when(dockerfileGitHubUtil).modifyOnGithub(any(), eq("image-tag"), eq("image")
, eq("tag"), anyString(), anyString());

//Both Dockerfiles modified
doReturn(true).when(dockerfileGitHubUtil).modifyContentOnGithub(eq(forkedRepoContent1),
eq("image-tag"),
eq("image"),
eq("tag"),
eq(null),
eq(null));
doReturn(true).when(dockerfileGitHubUtil).modifyContentOnGithub(eq(forkedRepoContent2),
eq("image-tag"),
eq("image"),
eq("tag"),
eq(null),
eq(null));

doNothing().when(rateLimiter).consume();

dockerfileGitHubUtil.changeDockerfiles(ns, pathToDockerfilesInParentRepo,
Expand All @@ -873,13 +880,153 @@ 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),
eq("image-tag"), eq(forkedRepo), any(), eq(rateLimiter));
}

@Test
public void testPullRequestBlockNotExecutedWhenDockerfileIsNotModified() throws Exception {
Map<String, Object> nsMap = ImmutableMap.of(Constants.IMG,
"image", Constants.TAG,
"tag", Constants.STORE,
"store");
Namespace ns = new Namespace(nsMap);
GitForkBranch gitForkBranch = new GitForkBranch("image", "tag", null, "");
Multimap<String, GitHubContentToProcess> pathToDockerfilesInParentRepo = HashMultimap.create();
pathToDockerfilesInParentRepo.put("repo", new GitHubContentToProcess(null, null, "df11"));

GHRepository forkedRepo = mock(GHRepository.class);
when(forkedRepo.isFork()).thenReturn(true);
when(forkedRepo.getFullName()).thenReturn("forkedrepo");

GHRepository parentRepo = mock(GHRepository.class);
when(parentRepo.getFullName()).thenReturn("repo");
when(forkedRepo.getParent()).thenReturn(parentRepo);

String defaultBranch = "default";
when(parentRepo.getDefaultBranch()).thenReturn(defaultBranch);
GHBranch parentBranch = mock(GHBranch.class);
String sha = "abcdef";
when(parentBranch.getSHA1()).thenReturn(sha);
when(parentRepo.getBranch(defaultBranch)).thenReturn(parentBranch);

gitHubUtil = mock(GitHubUtil.class);
dockerfileGitHubUtil = Mockito.spy(new DockerfileGitHubUtil(gitHubUtil));
InputStream stream = new ByteArrayInputStream("someContent".getBytes(StandardCharsets.UTF_8));
GHContent forkedRepoContent = mock(GHContent.class);
when(forkedRepoContent.read()).thenReturn(stream);

RateLimiter rateLimiter = mock(RateLimiter.class);
when(gitHubUtil.tryRetrievingContent(eq(forkedRepo),
eq("df11"), eq("image-tag"))).thenReturn(forkedRepoContent);

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

doNothing().when(rateLimiter).consume();

dockerfileGitHubUtil.changeDockerfiles(ns, pathToDockerfilesInParentRepo,
new GitHubContentToProcess(forkedRepo, parentRepo, ""), new ArrayList<>(),
gitForkBranch, rateLimiter);

// Dockerfile retrieved from the repo
verify(dockerfileGitHubUtil).tryRetrievingContent(eq(forkedRepo),
eq("df11"), eq("image-tag"));

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

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

@Test
public void testPullRequestCreatedWhenAnyDockerfileIsModified() throws Exception {
Map<String, Object> nsMap = ImmutableMap.of(Constants.IMG,
"image", Constants.TAG,
"tag", Constants.STORE,
"store");
Namespace ns = new Namespace(nsMap);
GitForkBranch gitForkBranch = new GitForkBranch("image", "tag", null, "");
Multimap<String, GitHubContentToProcess> pathToDockerfilesInParentRepo = HashMultimap.create();
pathToDockerfilesInParentRepo.put("repo", new GitHubContentToProcess(null, null, "df11"));
pathToDockerfilesInParentRepo.put("repo", new GitHubContentToProcess(null, null, "df12"));

GHRepository forkedRepo = mock(GHRepository.class);
when(forkedRepo.isFork()).thenReturn(true);
when(forkedRepo.getFullName()).thenReturn("forkedrepo");

GHRepository parentRepo = mock(GHRepository.class);
when(parentRepo.getFullName()).thenReturn("repo");
when(forkedRepo.getParent()).thenReturn(parentRepo);

String defaultBranch = "default";
when(parentRepo.getDefaultBranch()).thenReturn(defaultBranch);
GHBranch parentBranch = mock(GHBranch.class);
String sha = "abcdef";
when(parentBranch.getSHA1()).thenReturn(sha);
when(parentRepo.getBranch(defaultBranch)).thenReturn(parentBranch);

gitHubUtil = mock(GitHubUtil.class);
dockerfileGitHubUtil = Mockito.spy(new DockerfileGitHubUtil(gitHubUtil));
InputStream stream = new ByteArrayInputStream("someContent".getBytes(StandardCharsets.UTF_8));
GHContent forkedRepoContent = mock(GHContent.class);
when(forkedRepoContent.read()).thenReturn(stream);

RateLimiter rateLimiter = mock(RateLimiter.class);
when(gitHubUtil.tryRetrievingContent(eq(forkedRepo),
eq("df11"), eq("image-tag"))).thenReturn(forkedRepoContent);
GHContent forkedRepoContent2 = mock(GHContent.class);
when(forkedRepoContent2.read()).thenReturn(stream);
when(gitHubUtil.tryRetrievingContent(eq(forkedRepo),
eq("df12"), eq("image-tag"))).thenReturn(forkedRepoContent2);

//First dockerfile not modified
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).modifyContentOnGithub(eq(forkedRepoContent2),
eq("image-tag"),
eq("image"),
eq("tag"),
eq(null),
eq(null));

doNothing().when(rateLimiter).consume();

dockerfileGitHubUtil.changeDockerfiles(ns, pathToDockerfilesInParentRepo,
new GitHubContentToProcess(forkedRepo, parentRepo, ""), new ArrayList<>(),
gitForkBranch, rateLimiter);

// Both Dockerfiles retrieved from the same repo
verify(dockerfileGitHubUtil).tryRetrievingContent(eq(forkedRepo),
eq("df11"), eq("image-tag"));
verify(dockerfileGitHubUtil).tryRetrievingContent(eq(forkedRepo),
eq("df12"), eq("image-tag"));

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

// PR created
verify(dockerfileGitHubUtil).createPullReq(eq(parentRepo),
eq("image-tag"), eq(forkedRepo), any(), eq(rateLimiter));
}

@DataProvider
public Object[][] fromInstructionWithIgnoreStringData() {
return new Object[][] {
Expand Down

0 comments on commit 15a6327

Please sign in to comment.