From d7114792642fd4a20443288eb3deed59dd6a3609 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Tue, 19 Feb 2013 15:24:34 +0100 Subject: [PATCH] Fix quality flaws --- .../filesystem/DefaultModuleFileSystem.java | 2 + .../maven/AbstractMavenPluginExecutor.java | 42 +++++++++---------- .../DeprecatedFileSystemAdapterTest.java | 20 +++++++++ .../AbstractMavenPluginExecutorTest.java | 26 ++++++++++-- 4 files changed, 65 insertions(+), 25 deletions(-) diff --git a/sonar-batch/src/main/java/org/sonar/batch/scan/filesystem/DefaultModuleFileSystem.java b/sonar-batch/src/main/java/org/sonar/batch/scan/filesystem/DefaultModuleFileSystem.java index 4d29ee9f8ab..8ba27fadce5 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/scan/filesystem/DefaultModuleFileSystem.java +++ b/sonar-batch/src/main/java/org/sonar/batch/scan/filesystem/DefaultModuleFileSystem.java @@ -134,6 +134,8 @@ public class DefaultModuleFileSystem implements ModuleFileSystem { case TEST: applyFilters(result, context, filters, testDirs); break; + default: + throw new IllegalArgumentException("Unknown file type: " + type); } } return result; diff --git a/sonar-batch/src/main/java/org/sonar/batch/scan/maven/AbstractMavenPluginExecutor.java b/sonar-batch/src/main/java/org/sonar/batch/scan/maven/AbstractMavenPluginExecutor.java index d4e84e15831..c6c6d527b7c 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/scan/maven/AbstractMavenPluginExecutor.java +++ b/sonar-batch/src/main/java/org/sonar/batch/scan/maven/AbstractMavenPluginExecutor.java @@ -25,9 +25,7 @@ import org.sonar.api.batch.maven.MavenPluginHandler; import org.sonar.api.resources.Project; import org.sonar.api.utils.SonarException; import org.sonar.api.utils.TimeProfiler; -import org.sonar.batch.scan.maven.MavenProjectConverter; import org.sonar.batch.scan.filesystem.DefaultModuleFileSystem; -import org.sonar.batch.scan.maven.MavenPluginExecutor; /** * Abstract implementation of {@link org.sonar.batch.scan.maven.MavenPluginExecutor} to reduce duplications in concrete implementations for different Maven versions. @@ -38,26 +36,26 @@ public abstract class AbstractMavenPluginExecutor implements MavenPluginExecutor for (String goal : handler.getGoals()) { MavenPlugin plugin = MavenPlugin.getPlugin(project.getPom(), handler.getGroupId(), handler.getArtifactId()); execute(project, - fs, - getGoal(handler.getGroupId(), handler.getArtifactId(), (plugin != null && plugin.getPlugin() != null ? plugin.getPlugin().getVersion() : null), goal)); + fs, + getGoal(handler.getGroupId(), handler.getArtifactId(), (plugin != null && plugin.getPlugin() != null ? plugin.getPlugin().getVersion() : null), goal)); } return handler; } public final void execute(Project project, DefaultModuleFileSystem fs, String goal) { - TimeProfiler profiler = new TimeProfiler().start("Execute " + goal); - ClassLoader currentClassLoader = Thread.currentThread().getContextClassLoader(); - try { - concreteExecute(project.getPom(), goal); - } catch (Exception e) { - throw new SonarException("Unable to execute maven plugin", e); - } finally { - // Reset original ClassLoader that may have been changed during Maven Execution (see SONAR-1800) - Thread.currentThread().setContextClassLoader(currentClassLoader); - profiler.stop(); - } - if (project.getPom() != null) { + TimeProfiler profiler = new TimeProfiler().start("Execute " + goal); + ClassLoader currentClassLoader = Thread.currentThread().getContextClassLoader(); + try { + concreteExecute(project.getPom(), goal); + } catch (Exception e) { + throw new SonarException("Unable to execute maven plugin", e); + } finally { + // Reset original ClassLoader that may have been changed during Maven Execution (see SONAR-1800) + Thread.currentThread().setContextClassLoader(currentClassLoader); + profiler.stop(); + } + MavenProjectConverter.synchronizeFileSystem(project.getPom(), fs); } } @@ -67,12 +65,12 @@ public abstract class AbstractMavenPluginExecutor implements MavenPluginExecutor static String getGoal(String groupId, String artifactId, String version, String goal) { String defaultVersion = (version == null ? "" : version); return new StringBuilder() - .append(groupId).append(":") - .append(artifactId).append(":") - .append(defaultVersion) - .append(":") - .append(goal) - .toString(); + .append(groupId).append(":") + .append(artifactId).append(":") + .append(defaultVersion) + .append(":") + .append(goal) + .toString(); } } diff --git a/sonar-batch/src/test/java/org/sonar/batch/scan/filesystem/DeprecatedFileSystemAdapterTest.java b/sonar-batch/src/test/java/org/sonar/batch/scan/filesystem/DeprecatedFileSystemAdapterTest.java index fae9adc1b79..56a2cb85f20 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/scan/filesystem/DeprecatedFileSystemAdapterTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/scan/filesystem/DeprecatedFileSystemAdapterTest.java @@ -19,15 +19,24 @@ */ package org.sonar.batch.scan.filesystem; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; import org.mockito.Mockito; import org.sonar.api.resources.Project; +import java.io.File; +import java.io.IOException; + import static org.fest.assertions.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; public class DeprecatedFileSystemAdapterTest { + @Rule + public TemporaryFolder temp = new TemporaryFolder(); + @Test public void should_wrap_module_file_system() { DefaultModuleFileSystem target = mock(DefaultModuleFileSystem.class, Mockito.RETURNS_SMART_NULLS); @@ -48,4 +57,15 @@ public class DeprecatedFileSystemAdapterTest { assertThat(adapter.getBuildDir()).isNotNull(); verify(target).buildDir(); } + + @Test + public void should_create_default_build_dir() throws IOException { + File workingDir = temp.newFile("work"); + DefaultModuleFileSystem target = mock(DefaultModuleFileSystem.class); + when(target.workingDir()).thenReturn(workingDir); + DeprecatedFileSystemAdapter adapter = new DeprecatedFileSystemAdapter(target, new Project("my-project")); + + File buildDir = adapter.getBuildDir(); + assertThat(buildDir.getParentFile().getCanonicalPath()).isEqualTo(workingDir.getCanonicalPath()); + } } diff --git a/sonar-batch/src/test/java/org/sonar/batch/scan/maven/AbstractMavenPluginExecutorTest.java b/sonar-batch/src/test/java/org/sonar/batch/scan/maven/AbstractMavenPluginExecutorTest.java index 754ec60381d..41bb769edaa 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/scan/maven/AbstractMavenPluginExecutorTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/scan/maven/AbstractMavenPluginExecutorTest.java @@ -25,7 +25,6 @@ import org.sonar.api.batch.maven.MavenPlugin; import org.sonar.api.batch.maven.MavenPluginHandler; import org.sonar.api.resources.Project; import org.sonar.batch.scan.filesystem.DefaultModuleFileSystem; -import org.sonar.batch.scan.maven.AbstractMavenPluginExecutor; import java.io.File; @@ -34,21 +33,27 @@ import static org.junit.Assert.assertThat; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyList; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; public class AbstractMavenPluginExecutorTest { @Test - public void pluginVersionIsOptional() { + public void plugin_version_should_be_optional() { assertThat(AbstractMavenPluginExecutor.getGoal("group", "artifact", null, "goal"), is("group:artifact::goal")); } + @Test + public void test_plugin_version() { + assertThat(AbstractMavenPluginExecutor.getGoal("group", "artifact", "3.54", "goal"), is("group:artifact:3.54:goal")); + } + /** * The maven plugin sometimes changes the project structure (for example mvn build-helper:add-source). These changes * must be applied to the internal structure. */ @Test - public void shouldUpdateProjectAfterExecution() { + public void should_reset_file_system_after_execution() { AbstractMavenPluginExecutor executor = new AbstractMavenPluginExecutor() { @Override public void concreteExecute(MavenProject pom, String goal) throws Exception { @@ -66,6 +71,21 @@ public class AbstractMavenPluginExecutorTest { verify(fs).resetDirs(any(File.class), any(File.class), anyList(), anyList(), anyList()); } + @Test + public void should_ignore_non_maven_projects() { + AbstractMavenPluginExecutor executor = new AbstractMavenPluginExecutor() { + @Override + public void concreteExecute(MavenProject pom, String goal) throws Exception { + pom.addCompileSourceRoot("src/java"); + } + }; + Project foo = new Project("foo"); + DefaultModuleFileSystem fs = mock(DefaultModuleFileSystem.class); + executor.execute(foo, fs, new AddSourceMavenPluginHandler()); + + verify(fs, never()).resetDirs(any(File.class), any(File.class), anyList(), anyList(), anyList()); + } + static class AddSourceMavenPluginHandler implements MavenPluginHandler { public String getGroupId() { return "fake"; -- 2.39.5