From b3cf62536654bedd099797131199b5ca5fce99e0 Mon Sep 17 00:00:00 2001 From: Adrien Lecharpentier Date: Mon, 14 Oct 2024 14:46:38 +0200 Subject: [PATCH] Plugin repository ownership based on repository name (#554) --- .../scoring/probes/CodeOwnershipProbe.java | 5 +- .../probes/SCMLinkValidationProbe.java | 16 ++-- .../probes/CodeOwnershipProbeTest.java | 76 ++++++++++++++++++- .../probes/SCMLinkValidationProbeTest.java | 8 +- 4 files changed, 88 insertions(+), 17 deletions(-) diff --git a/core/src/main/java/io/jenkins/pluginhealth/scoring/probes/CodeOwnershipProbe.java b/core/src/main/java/io/jenkins/pluginhealth/scoring/probes/CodeOwnershipProbe.java index 65f66b7ba..964c9b291 100644 --- a/core/src/main/java/io/jenkins/pluginhealth/scoring/probes/CodeOwnershipProbe.java +++ b/core/src/main/java/io/jenkins/pluginhealth/scoring/probes/CodeOwnershipProbe.java @@ -59,8 +59,9 @@ protected ProbeResult doApply(Plugin plugin, ProbeContext context) { .map(file -> { try { return Files.readAllLines(file).stream() - .anyMatch(line -> line.contains( - "@jenkinsci/%s-plugin-developers".formatted(plugin.getName()))) + .anyMatch(line -> line.contains("@jenkinsci/%s-developers" + .formatted(context.getRepositoryName() + .orElse("NOT_VALID")))) ? this.success("CODEOWNERS file is valid.") : this.success("CODEOWNERS file is not set correctly."); } catch (IOException ex) { diff --git a/core/src/main/java/io/jenkins/pluginhealth/scoring/probes/SCMLinkValidationProbe.java b/core/src/main/java/io/jenkins/pluginhealth/scoring/probes/SCMLinkValidationProbe.java index 8af521f9a..a9ec3f344 100644 --- a/core/src/main/java/io/jenkins/pluginhealth/scoring/probes/SCMLinkValidationProbe.java +++ b/core/src/main/java/io/jenkins/pluginhealth/scoring/probes/SCMLinkValidationProbe.java @@ -1,7 +1,7 @@ /* * MIT License * - * Copyright (c) 2023 Jenkins Infra + * Copyright (c) 2023-2024 Jenkins Infra * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -21,7 +21,6 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - package io.jenkins.pluginhealth.scoring.probes; import java.io.FileInputStream; @@ -56,7 +55,7 @@ public class SCMLinkValidationProbe extends Probe { public static final int ORDER = UpdateCenterPluginPublicationProbe.ORDER + 100; public static final String KEY = "scm"; private static final Logger LOGGER = LoggerFactory.getLogger(SCMLinkValidationProbe.class); - private static final String GH_REGEXP = "https://(?[^/]*)/(?jenkinsci/[^/]*)"; + private static final String GH_REGEXP = "https://(?[^/]*)/(?jenkinsci/[^/]*-plugin)"; public static final Pattern GH_PATTERN = Pattern.compile(GH_REGEXP); @Override @@ -88,7 +87,8 @@ private ProbeResult fromSCMLink(ProbeContext context, String scm, String pluginN } try { context.getGitHub().getRepository(matcher.group("repo")); - Optional pluginPathInRepository = findPluginPom(context.getScmRepository().get(), pluginName); + Optional pluginPathInRepository = + findPluginPom(context.getScmRepository().get(), pluginName); Optional folderPath = pluginPathInRepository.map(path -> path.getParent()); if (folderPath.isEmpty()) { return this.success(String.format("No valid POM file found in %s plugin.", pluginName)); @@ -117,11 +117,9 @@ private Optional findPluginPom(Path directory, String pluginName) { * The `maxDepth` is 3 because a lot of plugins aren't located deeper than /plugins/. * If the `maxDepth` is more than 3, we will be navigating the `src/main/java/io/jenkins/plugins/artifactId/` folder. * */ - try (Stream paths = Files.find(directory, 3, (path, $) -> - "pom.xml".equals(path.getFileName().toString()))) { - return paths - .filter(pom -> pomFileMatchesPlugin(pom, pluginName)) - .findFirst(); + try (Stream paths = Files.find( + directory, 3, (path, $) -> "pom.xml".equals(path.getFileName().toString()))) { + return paths.filter(pom -> pomFileMatchesPlugin(pom, pluginName)).findFirst(); } catch (IOException e) { LOGGER.error("Could not browse the folder during probe {}.", pluginName, e); } diff --git a/core/src/test/java/io/jenkins/pluginhealth/scoring/probes/CodeOwnershipProbeTest.java b/core/src/test/java/io/jenkins/pluginhealth/scoring/probes/CodeOwnershipProbeTest.java index d534f028a..a000bdbab 100644 --- a/core/src/test/java/io/jenkins/pluginhealth/scoring/probes/CodeOwnershipProbeTest.java +++ b/core/src/test/java/io/jenkins/pluginhealth/scoring/probes/CodeOwnershipProbeTest.java @@ -71,7 +71,7 @@ void shouldDetectCodeOwnershipFileWithInvalidContent() throws IOException { final Plugin plugin = mock(Plugin.class); final ProbeContext ctx = mock(ProbeContext.class); - when(plugin.getName()).thenReturn("new-super"); + when(ctx.getRepositoryName()).thenReturn(Optional.of("new-super-plugin")); { final Path repo = Files.createTempDirectory(getClass().getName()); @@ -102,6 +102,15 @@ void shouldDetectCodeOwnershipFileWithInvalidContent() throws IOException { """); when(ctx.getScmRepository()).thenReturn(Optional.of(repo)); } + { + final Path repo = Files.createTempDirectory(getClass().getName()); + final Path docs = Files.createDirectory(repo.resolve(".github")); + final Path codeowners = Files.createFile(docs.resolve("CODEOWNERS")); + Files.writeString(codeowners, """ + * @alecharp + """); + when(ctx.getScmRepository()).thenReturn(Optional.of(repo)); + } final CodeOwnershipProbe probe = getSpy(); @@ -117,6 +126,10 @@ void shouldDetectCodeOwnershipFileWithInvalidContent() throws IOException { .usingRecursiveComparison() .comparingOnlyFields("id", "status", "message") .isEqualTo(ProbeResult.success(probe.key(), "CODEOWNERS file is not set correctly.", 0)); + assertThat(probe.apply(plugin, ctx)) + .usingRecursiveComparison() + .comparingOnlyFields("id", "status", "message") + .isEqualTo(ProbeResult.success(probe.key(), "CODEOWNERS file is not set correctly.", 0)); } @Test @@ -124,7 +137,7 @@ void shouldDetectCodeOwnershipFileWithValidTeam() throws IOException { final Plugin plugin = mock(Plugin.class); final ProbeContext ctx = mock(ProbeContext.class); - when(plugin.getName()).thenReturn("sample"); + when(ctx.getRepositoryName()).thenReturn(Optional.of("sample-plugin")); { final Path repo = Files.createTempDirectory(getClass().getName()); @@ -171,4 +184,63 @@ void shouldDetectCodeOwnershipFileWithValidTeam() throws IOException { .comparingOnlyFields("id", "status", "message") .isEqualTo(ProbeResult.success(probe.key(), "CODEOWNERS file is valid.", 0)); } + + @Test + void shouldDetectCodeOwnershipFileWithValidTeamAndMore() throws IOException { + final Plugin plugin = mock(Plugin.class); + final ProbeContext ctx = mock(ProbeContext.class); + + when(ctx.getRepositoryName()).thenReturn(Optional.of("sample-plugin")); + + { + final Path repo = Files.createTempDirectory(getClass().getName()); + final Path codeowners = Files.createFile(repo.resolve("CODEOWNERS")); + Files.writeString( + codeowners, + """ + * @jenkinsci/sample-plugin-developers + * @alecharp @jenkinsci + """); + when(ctx.getScmRepository()).thenReturn(Optional.of(repo)); + } + { + final Path repo = Files.createTempDirectory(getClass().getName()); + final Path github = Files.createDirectory(repo.resolve(".github")); + final Path codeowners = Files.createFile(github.resolve("CODEOWNERS")); + Files.writeString( + codeowners, + """ + * @jenkinsci/sample-plugin-developers + * @alecharp @jenkinsci + """); + when(ctx.getScmRepository()).thenReturn(Optional.of(repo)); + } + { + final Path repo = Files.createTempDirectory(getClass().getName()); + final Path docs = Files.createDirectory(repo.resolve("docs")); + final Path codeowners = Files.createFile(docs.resolve("CODEOWNERS")); + Files.writeString( + codeowners, + """ + * @jenkinsci/sample-plugin-developers + * @alecharp @jenkinsci + """); + when(ctx.getScmRepository()).thenReturn(Optional.of(repo)); + } + + final CodeOwnershipProbe probe = getSpy(); + + assertThat(probe.apply(plugin, ctx)) + .usingRecursiveComparison() + .comparingOnlyFields("id", "status", "message") + .isEqualTo(ProbeResult.success(probe.key(), "CODEOWNERS file is valid.", 0)); + assertThat(probe.apply(plugin, ctx)) + .usingRecursiveComparison() + .comparingOnlyFields("id", "status", "message") + .isEqualTo(ProbeResult.success(probe.key(), "CODEOWNERS file is valid.", 0)); + assertThat(probe.apply(plugin, ctx)) + .usingRecursiveComparison() + .comparingOnlyFields("id", "status", "message") + .isEqualTo(ProbeResult.success(probe.key(), "CODEOWNERS file is valid.", 0)); + } } diff --git a/core/src/test/java/io/jenkins/pluginhealth/scoring/probes/SCMLinkValidationProbeTest.java b/core/src/test/java/io/jenkins/pluginhealth/scoring/probes/SCMLinkValidationProbeTest.java index 7139255b9..46b7478eb 100644 --- a/core/src/test/java/io/jenkins/pluginhealth/scoring/probes/SCMLinkValidationProbeTest.java +++ b/core/src/test/java/io/jenkins/pluginhealth/scoring/probes/SCMLinkValidationProbeTest.java @@ -99,7 +99,7 @@ void shouldRecognizeCorrectGitHubUrl() throws IOException { final Plugin plugin = mock(Plugin.class); final ProbeContext ctx = mock(ProbeContext.class); final GitHub gh = mock(GitHub.class); - final String repositoryName = "jenkinsci/test-repo"; + final String repositoryName = "jenkinsci/test-repo-plugin"; when(plugin.getName()).thenReturn("test-repo"); when(plugin.getScm()).thenReturn("https://github.com/" + repositoryName); @@ -124,7 +124,7 @@ void shouldRecognizeInvalidGitHubUrl() throws Exception { final Plugin plugin = mock(Plugin.class); final ProbeContext ctx = mock(ProbeContext.class); final GitHub gh = mock(GitHub.class); - final String repositoryName = "jenkinsci/this-is-not-going-to-work"; + final String repositoryName = "jenkinsci/this-is-not-going-to-work-plugin"; when(plugin.getScm()).thenReturn("https://github.com/" + repositoryName); when(plugin.getName()).thenReturn("foo"); @@ -150,13 +150,13 @@ void shouldBeAbleToFindPluginInModule() throws IOException { final GitHub gh = mock(GitHub.class); final GHRepository ghRepo = mock(GHRepository.class); - when(plugin.getScm()).thenReturn("https://github.com/jenkinsci/this-is-fine"); + when(plugin.getScm()).thenReturn("https://github.com/jenkinsci/this-is-fine-plugin"); when(plugin.getName()).thenReturn("test-repo"); when(ctx.getScmRepository()) .thenReturn(Optional.of(Path.of("src/test/resources/jenkinsci/test-repo/test-nested-dir-1"))); when(ctx.getGitHub()).thenReturn(gh); - when(gh.getRepository("jenkinsci/this-is-fine")).thenReturn(ghRepo); + when(gh.getRepository("jenkinsci/this-is-fine-plugin")).thenReturn(ghRepo); final SCMLinkValidationProbe probe = getSpy(); final ProbeResult result = probe.apply(plugin, ctx);