From 79f71bd17032bcf9f80dda69c62a99315aabf9d2 Mon Sep 17 00:00:00 2001 From: Avimanyu Mukhopadhyay Date: Tue, 4 Apr 2023 13:41:38 -0700 Subject: [PATCH] Addressing PR comments --- .../utils/DockerfileGitHubUtil.java | 18 +++++++++------- .../utils/DockerfileGitHubUtilTest.java | 21 ++++++++----------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java index 1b6943a6..83148582 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java @@ -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, @@ -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; } } diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtilTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtilTest.java index 8dcebf4a..e530c7bc 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtilTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtilTest.java @@ -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); @@ -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"), @@ -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), @@ -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); @@ -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"), @@ -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)); } @@ -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); @@ -994,7 +991,7 @@ 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"), @@ -1002,7 +999,7 @@ public void testPullRequestCreatedWhenAnyDockerfileIsModified() throws Exception 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"), @@ -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