Skip to content

Commit

Permalink
SCANMAVEN-214 Disable sonar.scanAll when sonar.tests is specified (#216)
Browse files Browse the repository at this point in the history
  • Loading branch information
ADarko22 authored May 14, 2024
1 parent e1c628e commit 09625de
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ public class MavenProjectConverter {
private final Set<Path> skippedBasedDirs = new HashSet<>();

private boolean sourceDirsIsOverridden = false;
private boolean testDirsIsOverridden = false;

/**
* This field is introduced to keep track of the root project in multi-module projects and can be used to decide
Expand All @@ -148,6 +149,10 @@ public boolean isSourceDirsOverridden() {
return sourceDirsIsOverridden;
}

public boolean isTestDirsOverridden() {
return testDirsIsOverridden;
}

public Properties getEnvProperties() {
return new Properties(envProperties);
}
Expand Down Expand Up @@ -585,6 +590,7 @@ private List<File> sourcePaths(MavenProject pom, String propertyKey, Collection<
filesOrDirs = resolvePaths(paths, pom.getBasedir());
userDefined = true;
sourceDirsIsOverridden |= propertyKey.equals(ScanProperties.PROJECT_SOURCE_DIRS);
testDirsIsOverridden |= propertyKey.equals(ScanProperties.PROJECT_TEST_DIRS);
} else {
removeTarget(pom, mavenPaths);
filesOrDirs = resolvePaths(mavenPaths, pom.getBasedir());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ public void execute() throws MojoExecutionException {
checkSQVersion();
}


if (log.isDebugEnabled()) {
scanner.setGlobalProperty("sonar.verbose", "true");
}
Expand All @@ -91,7 +90,6 @@ public void execute() throws MojoExecutionException {
}
}


// TODO remove this workaround when discovering if the sevrer is SC or SQ is available through the API
private boolean isSonarCloudUsed() {
return session.getProjects().stream()
Expand All @@ -116,20 +114,22 @@ Map<String, String> collectProperties()
break;
}
}

if (topLevelProject == null) {
throw new IllegalStateException("Maven session does not declare a top level project");
}

Properties userProperties = session.getUserProperties();
Map<String, String> props = mavenProjectConverter.configure(sortedProjects, topLevelProject, userProperties);
props.putAll(propertyDecryptor.decryptProperties(props));
if (shouldCollectAllSources(userProperties)) {
log.info("Parameter " + MavenScannerProperties.PROJECT_SCAN_ALL_SOURCES + " is enabled. The scanner will attempt to collect additional sources.");
if (!mavenProjectConverter.isSourceDirsOverridden()) {
collectAllSources(props);
if (mavenProjectConverter.isSourceDirsOverridden()) {
log.warn(notCollectingAdditionalSourcesBecauseOf(ScanProperties.PROJECT_SOURCE_DIRS));
} else if (mavenProjectConverter.isTestDirsOverridden()) {
log.warn(notCollectingAdditionalSourcesBecauseOf(ScanProperties.PROJECT_TEST_DIRS));
} else {
String warning = "Parameter " + MavenScannerProperties.PROJECT_SCAN_ALL_SOURCES + " is enabled but " +
"the scanner will not collect additional sources because sonar.sources has been overridden.";
log.warn(warning);
collectAllSources(props);
}
}

Expand All @@ -140,7 +140,13 @@ private static boolean shouldCollectAllSources(Properties userProperties) {
return Boolean.TRUE.equals(Boolean.parseBoolean(userProperties.getProperty(MavenScannerProperties.PROJECT_SCAN_ALL_SOURCES)));
}

private void collectAllSources(Map<String, String> props) {
private static String notCollectingAdditionalSourcesBecauseOf(String overriddenProperty) {
return "Parameter " + MavenScannerProperties.PROJECT_SCAN_ALL_SOURCES + " is enabled but " +
"the scanner will not collect additional sources because " + overriddenProperty + " has been overridden.";
}

@VisibleForTesting
void collectAllSources(Map<String, String> props) {
String projectBasedir = props.get(ScanProperties.PROJECT_BASEDIR);
// Exclude the files and folders covered by sonar.sources and sonar.tests (and sonar.exclusions) as computed by the MavenConverter
// Combine all the sonar.sources at the top-level and by module
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,7 @@
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.mockito.InOrder;
import org.mockito.Mock;
import org.mockito.MockedStatic;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
import org.mockito.*;
import org.sonarsource.scanner.api.EmbeddedScanner;
import org.sonarsource.scanner.api.ScanProperties;
import org.sonatype.plexus.components.sec.dispatcher.SecDispatcher;
Expand All @@ -51,14 +47,7 @@
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.contains;
import static org.mockito.Mockito.inOrder;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
import static org.mockito.Mockito.*;
import static org.sonarsource.scanner.maven.bootstrap.ScannerBootstrapper.UNSUPPORTED_BELOW_SONARQUBE_56_MESSAGE;

class ScannerBootstrapperTest {
Expand All @@ -84,7 +73,6 @@ class ScannerBootstrapperTest {

private Map<String, String> projectProperties;


@BeforeEach
public void setUp()
throws MojoExecutionException, IOException {
Expand All @@ -108,14 +96,14 @@ public void setUp()
javascriptResource.toFile().createNewFile();
projectProperties.put(ScanProperties.PROJECT_SOURCE_DIRS, sourceMainDirs.toFile().toString() + "," + pom.toFile().toString());


when(mavenProjectConverter.configure(any(), any(), any())).thenReturn(projectProperties);
when(mavenProjectConverter.getEnvProperties()).thenReturn(new Properties());
when(rootProject.getProperties()).thenReturn(new Properties());

when(scanner.mask(anyString())).thenReturn(scanner);
when(scanner.unmask(anyString())).thenReturn(scanner);
scannerBootstrapper = new ScannerBootstrapper(log, session, scanner, mavenProjectConverter, new PropertyDecryptor(log, securityDispatcher));
ScannerBootstrapper scannerBootstrapperTmp = new ScannerBootstrapper(log, session, scanner, mavenProjectConverter, new PropertyDecryptor(log, securityDispatcher));
scannerBootstrapper = spy(scannerBootstrapperTmp);
}

@Test
Expand Down Expand Up @@ -174,6 +162,7 @@ void scanAll_property_is_detected_and_applied() throws MojoExecutionException {
assertThat(sourceDirs[0]).endsWith(Paths.get("src", "main", "java").toString());
assertThat(sourceDirs[1]).endsWith(Paths.get("pom.xml").toString());
verify(log, never()).info("Parameter sonar.maven.scanAll is enabled. The scanner will attempt to collect additional sources.");
verify(scannerBootstrapper, never()).collectAllSources(any());

// When sonar.scanner.scanAll is set explicitly to false
Properties withScanAllSetToFalse = new Properties();
Expand All @@ -186,7 +175,7 @@ void scanAll_property_is_detected_and_applied() throws MojoExecutionException {
assertThat(sourceDirs[0]).endsWith(Paths.get("src", "main", "java").toString());
assertThat(sourceDirs[1]).endsWith(Paths.get("pom.xml").toString());
verify(log, never()).info("Parameter sonar.maven.scanAll is enabled. The scanner will attempt to collect additional sources.");

verify(scannerBootstrapper, never()).collectAllSources(any());

// When sonar.scanner.scanAll is set explicitly to true
Properties withScanAllSetToTrue = new Properties();
Expand All @@ -200,6 +189,7 @@ void scanAll_property_is_detected_and_applied() throws MojoExecutionException {
assertThat(sourceDirs[1]).endsWith(Paths.get("pom.xml").toString());
assertThat(sourceDirs[2]).endsWith(Paths.get("src", "main", "resources", "index.js").toString());
verify(log, times(1)).info("Parameter sonar.maven.scanAll is enabled. The scanner will attempt to collect additional sources.");
verify(scannerBootstrapper, times(1)).collectAllSources(any());
}

@Test
Expand All @@ -220,6 +210,28 @@ void should_not_collect_all_sources_when_sonar_sources_is_overridden() throws Mo

verify(log, times(1)).info("Parameter sonar.maven.scanAll is enabled. The scanner will attempt to collect additional sources.");
verify(log, times(1)).warn("Parameter sonar.maven.scanAll is enabled but the scanner will not collect additional sources because sonar.sources has been overridden.");
verify(scannerBootstrapper, never()).collectAllSources(any());
}

@Test
void should_not_collect_all_sources_when_sonar_tests_is_overridden() throws MojoExecutionException {
// When sonar.scanner.scanAll is set explicitly to true
Properties withScanAllSetToTrue = new Properties();
withScanAllSetToTrue.put(MavenScannerProperties.PROJECT_SCAN_ALL_SOURCES, "true");
when(session.getUserProperties()).thenReturn(withScanAllSetToTrue);
// Return the expected directory and notify of overriding
projectProperties.put(ScanProperties.PROJECT_TEST_DIRS, Paths.get("src", "test", "resources").toFile().toString());
when(mavenProjectConverter.isTestDirsOverridden()).thenReturn(true);

Map<String, String> collectedProperties = scannerBootstrapper.collectProperties();
assertThat(collectedProperties).containsKey(ScanProperties.PROJECT_TEST_DIRS);
String[] sourceDirs = collectedProperties.get(ScanProperties.PROJECT_TEST_DIRS).split(",");
assertThat(sourceDirs).hasSize(1);
assertThat(sourceDirs[0]).endsWith(Paths.get("src", "test", "resources").toString());

verify(log, times(1)).info("Parameter sonar.maven.scanAll is enabled. The scanner will attempt to collect additional sources.");
verify(log, times(1)).warn("Parameter sonar.maven.scanAll is enabled but the scanner will not collect additional sources because sonar.tests has been overridden.");
verify(scannerBootstrapper, never()).collectAllSources(any());
}

@Test
Expand Down

0 comments on commit 09625de

Please sign in to comment.