Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions its/plugin/tests/src/test/java/com/sonar/it/java/JspTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,13 @@ public void should_transpile_jsp() throws Exception {
.matches(patternWithLiteralDot("""
Telemetry java.analysis.generated.success.size_chars: \\d{5}
Telemetry java.analysis.generated.success.time_ms: \\d+
Telemetry java.analysis.generated.success.type_error_count: 0
Telemetry java.analysis.main.success.size_chars: 969
Telemetry java.analysis.main.success.time_ms: \\d+
Telemetry java.analysis.main.success.type_error_count: 0
Telemetry java.analysis.test.success.size_chars: 20
Telemetry java.analysis.test.success.time_ms: \\d+
Telemetry java.analysis.test.success.type_error_count: 0
Telemetry java.dependency.lombok: absent
Telemetry java.dependency.spring-boot: absent
Telemetry java.dependency.spring-web: absent
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ private void executeAndAssertBuild(MavenBuild build, String projectKey) {
.matches(patternWithLiteralDot("""
Telemetry java.analysis.main.success.size_chars: \\d{4}
Telemetry java.analysis.main.success.time_ms: \\d+
Telemetry java.analysis.main.success.type_error_count: 0
Telemetry java.dependency.lombok: absent
Telemetry java.dependency.spring-boot: absent
Telemetry java.dependency.spring-web: 5.3.18
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,11 @@ public void simpleScan(InputFile inputFile, JParserConfig.Result result, Consume
modifyCompilationUnit.accept(ast);
visitor.visitFile(ast, sonarComponents != null && sonarComponents.fileCanBeSkipped(inputFile));
String path = inputFile.toString();
collectUndefinedTypes(path, ast.sema.undefinedTypes());
Set<JProblem> undefinedTypes = ast.sema.undefinedTypes();
collectUndefinedTypes(path, undefinedTypes);
cleanUp.accept(ast);
telemetryAnalysisKeys = javaAnalysisKeys.success();
telemetry.aggregateAsCounter(javaAnalysisKeys.success().typeErrorCountKey(), undefinedTypes.size());
} catch (RecognitionException e) {
checkInterrupted(e);
LOG.error(String.format(LOG_ERROR_UNABLE_TO_PARSE_FILE, inputFile));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,21 @@ public enum TelemetryKey {
JAVA_IS_AUTOSCAN("java.is_autoscan"),
JAVA_ANALYSIS_MAIN_SUCCESS_SIZE_CHARS("java.analysis.main.success.size_chars"),
JAVA_ANALYSIS_MAIN_SUCCESS_TIME_MS("java.analysis.main.success.time_ms"),
JAVA_ANALYSIS_MAIN_SUCCESS_TYPE_ERROR_COUNT("java.analysis.main.success.type_error_count"),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets change TelemetryKeyTest so that it makes sure all java.analysis keys are tested. This would involve extracting the lists in testCompoundKeys and validating and the product of their lengths is equal to the number of java.analysis keys.

JAVA_ANALYSIS_MAIN_PARSE_ERRORS_SIZE_CHARS("java.analysis.main.parse_errors.size_chars"),
JAVA_ANALYSIS_MAIN_PARSE_ERRORS_TIME_MS("java.analysis.main.parse_errors.time_ms"),
JAVA_ANALYSIS_MAIN_EXCEPTIONS_SIZE_CHARS("java.analysis.main.exceptions.size_chars"),
JAVA_ANALYSIS_MAIN_EXCEPTIONS_TIME_MS("java.analysis.main.exceptions.time_ms"),
JAVA_ANALYSIS_TEST_SUCCESS_SIZE_CHARS("java.analysis.test.success.size_chars"),
JAVA_ANALYSIS_TEST_SUCCESS_TIME_MS("java.analysis.test.success.time_ms"),
JAVA_ANALYSIS_TEST_SUCCESS_TYPE_ERROR_COUNT("java.analysis.test.success.type_error_count"),
JAVA_ANALYSIS_TEST_PARSE_ERRORS_SIZE_CHARS("java.analysis.test.parse_errors.size_chars"),
JAVA_ANALYSIS_TEST_PARSE_ERRORS_TIME_MS("java.analysis.test.parse_errors.time_ms"),
JAVA_ANALYSIS_TEST_EXCEPTIONS_SIZE_CHARS("java.analysis.test.exceptions.size_chars"),
JAVA_ANALYSIS_TEST_EXCEPTIONS_TIME_MS("java.analysis.test.exceptions.time_ms"),
JAVA_ANALYSIS_GENERATED_SUCCESS_SIZE_CHARS("java.analysis.generated.success.size_chars"),
JAVA_ANALYSIS_GENERATED_SUCCESS_TIME_MS("java.analysis.generated.success.time_ms"),
JAVA_ANALYSIS_GENERATED_SUCCESS_TYPE_ERROR_COUNT("java.analysis.generated.success.type_error_count"),
JAVA_ANALYSIS_GENERATED_PARSE_ERRORS_SIZE_CHARS("java.analysis.generated.parse_errors.size_chars"),
JAVA_ANALYSIS_GENERATED_PARSE_ERRORS_TIME_MS("java.analysis.generated.parse_errors.time_ms"),
JAVA_ANALYSIS_GENERATED_EXCEPTIONS_SIZE_CHARS("java.analysis.generated.exceptions.size_chars"),
Expand All @@ -48,26 +51,38 @@ public enum TelemetryKey {
JAVA_DEPENDENCY_SPRING_BOOT("java.dependency.spring-boot"),
JAVA_DEPENDENCY_SPRING_WEB("java.dependency.spring-web");

public record SpeedKeys(TelemetryKey sizeCharsKey, TelemetryKey timeMsKey) {
public interface SpeedKeys {
TelemetryKey sizeCharsKey();

TelemetryKey timeMsKey();
}

public record SizeAndTimeKeys(TelemetryKey sizeCharsKey, TelemetryKey timeMsKey) implements SpeedKeys {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer {} on the same line here and below.

}

public record SizeTimeAndTypeErrorKeys(TelemetryKey sizeCharsKey, TelemetryKey timeMsKey, TelemetryKey typeErrorCountKey) implements SpeedKeys {
}

public record JavaAnalysisKeys(SpeedKeys success, SpeedKeys parseErrors, SpeedKeys exceptions) {
public record JavaAnalysisKeys(SizeTimeAndTypeErrorKeys success, SpeedKeys parseErrors, SpeedKeys exceptions) {
}

public static final JavaAnalysisKeys JAVA_ANALYSIS_MAIN = new JavaAnalysisKeys(
new SpeedKeys(JAVA_ANALYSIS_MAIN_SUCCESS_SIZE_CHARS, JAVA_ANALYSIS_MAIN_SUCCESS_TIME_MS),
new SpeedKeys(JAVA_ANALYSIS_MAIN_PARSE_ERRORS_SIZE_CHARS, JAVA_ANALYSIS_MAIN_PARSE_ERRORS_TIME_MS),
new SpeedKeys(JAVA_ANALYSIS_MAIN_EXCEPTIONS_SIZE_CHARS, JAVA_ANALYSIS_MAIN_EXCEPTIONS_TIME_MS));
new SizeTimeAndTypeErrorKeys(JAVA_ANALYSIS_MAIN_SUCCESS_SIZE_CHARS, JAVA_ANALYSIS_MAIN_SUCCESS_TIME_MS,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's update the unit test for these data structures.

JAVA_ANALYSIS_MAIN_SUCCESS_TYPE_ERROR_COUNT),
new SizeAndTimeKeys(JAVA_ANALYSIS_MAIN_PARSE_ERRORS_SIZE_CHARS, JAVA_ANALYSIS_MAIN_PARSE_ERRORS_TIME_MS),
new SizeAndTimeKeys(JAVA_ANALYSIS_MAIN_EXCEPTIONS_SIZE_CHARS, JAVA_ANALYSIS_MAIN_EXCEPTIONS_TIME_MS));

public static final JavaAnalysisKeys JAVA_ANALYSIS_TEST = new JavaAnalysisKeys(
new SpeedKeys(JAVA_ANALYSIS_TEST_SUCCESS_SIZE_CHARS, JAVA_ANALYSIS_TEST_SUCCESS_TIME_MS),
new SpeedKeys(JAVA_ANALYSIS_TEST_PARSE_ERRORS_SIZE_CHARS, JAVA_ANALYSIS_TEST_PARSE_ERRORS_TIME_MS),
new SpeedKeys(JAVA_ANALYSIS_TEST_EXCEPTIONS_SIZE_CHARS, JAVA_ANALYSIS_TEST_EXCEPTIONS_TIME_MS));
new SizeTimeAndTypeErrorKeys(JAVA_ANALYSIS_TEST_SUCCESS_SIZE_CHARS, JAVA_ANALYSIS_TEST_SUCCESS_TIME_MS,
JAVA_ANALYSIS_TEST_SUCCESS_TYPE_ERROR_COUNT),
new SizeAndTimeKeys(JAVA_ANALYSIS_TEST_PARSE_ERRORS_SIZE_CHARS, JAVA_ANALYSIS_TEST_PARSE_ERRORS_TIME_MS),
new SizeAndTimeKeys(JAVA_ANALYSIS_TEST_EXCEPTIONS_SIZE_CHARS, JAVA_ANALYSIS_TEST_EXCEPTIONS_TIME_MS));

public static final JavaAnalysisKeys JAVA_ANALYSIS_GENERATED = new JavaAnalysisKeys(
new SpeedKeys(JAVA_ANALYSIS_GENERATED_SUCCESS_SIZE_CHARS, JAVA_ANALYSIS_GENERATED_SUCCESS_TIME_MS),
new SpeedKeys(JAVA_ANALYSIS_GENERATED_PARSE_ERRORS_SIZE_CHARS, JAVA_ANALYSIS_GENERATED_PARSE_ERRORS_TIME_MS),
new SpeedKeys(JAVA_ANALYSIS_GENERATED_EXCEPTIONS_SIZE_CHARS, JAVA_ANALYSIS_GENERATED_EXCEPTIONS_TIME_MS));
new SizeTimeAndTypeErrorKeys(JAVA_ANALYSIS_GENERATED_SUCCESS_SIZE_CHARS, JAVA_ANALYSIS_GENERATED_SUCCESS_TIME_MS,
JAVA_ANALYSIS_GENERATED_SUCCESS_TYPE_ERROR_COUNT),
new SizeAndTimeKeys(JAVA_ANALYSIS_GENERATED_PARSE_ERRORS_SIZE_CHARS, JAVA_ANALYSIS_GENERATED_PARSE_ERRORS_TIME_MS),
new SizeAndTimeKeys(JAVA_ANALYSIS_GENERATED_EXCEPTIONS_SIZE_CHARS, JAVA_ANALYSIS_GENERATED_EXCEPTIONS_TIME_MS));

private final String key;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.sonar.java.telemetry;

import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.function.Function;
import org.junit.jupiter.api.Test;
Expand All @@ -31,24 +32,33 @@
class TelemetryKeyTest {

@Test
void testKey() {
assertThat(TelemetryKey.JAVA_LANGUAGE_VERSION.key()).isEqualTo("java.language.version");
void test_TelemetryKey_names() {
for (TelemetryKey enumEntry : TelemetryKey.values()) {
assertThat(enumEntry.name()).isEqualTo(enumNameFromTelemetryName(enumEntry.key()));
}
}

@Test
void testCompoundKeys() {
for (String artifact : List.of("main.", "test.", "generated.")) {
for (String result : List.of("success.", "parse_errors.", "exceptions.")) {
for (String metric : List.of("size_chars", "time_ms")) {
String telemetryName = "java.analysis." + artifact + result + metric;
String javaName = telemetryName.toUpperCase().replace(".", "_");
TelemetryKey key = TelemetryKey.valueOf(javaName);
assertThat(key.key()).isEqualTo(telemetryName);
assertTelemetryNameExists("java.analysis." + artifact + result + metric);
}
}
assertTelemetryNameExists("java.analysis." + artifact + "success.type_error_count");
}
}

private static String enumNameFromTelemetryName(String telemetryName) {
return telemetryName.toUpperCase(Locale.ROOT).replaceAll("[.\\-]", "_");
}

private static void assertTelemetryNameExists(String telemetryName) {
TelemetryKey enumEntry = TelemetryKey.valueOf(enumNameFromTelemetryName(telemetryName));
assertThat(enumEntry.key()).isEqualTo(telemetryName);
}

@Test
void testAnalysisKeys() {
Map<String, JavaAnalysisKeys> artifacts = Map.of(
Expand All @@ -61,17 +71,21 @@ void testAnalysisKeys() {
"parse_errors.", JavaAnalysisKeys::parseErrors,
"exceptions.", JavaAnalysisKeys::exceptions
);
Map<String, Function<SpeedKeys,TelemetryKey>> metrics = Map.of(
Map<String, Function<SpeedKeys, TelemetryKey>> metrics = Map.of(
"size_chars", SpeedKeys::sizeCharsKey,
"time_ms", SpeedKeys::timeMsKey
);
artifacts.forEach((artifactName, artifact) -> results.forEach(
(resultName, result) -> metrics.forEach((
metricName, metric) -> {
String telemetryName = artifactName + resultName + metricName;
TelemetryKey tk = metric.apply(result.apply(artifact));
assertThat(tk.key()).isEqualTo(telemetryName);
})));
artifacts.forEach((artifactName, artifact) -> {
String typeErrorName = artifactName + "success.type_error_count";
assertThat(artifact.success().typeErrorCountKey().key()).isEqualTo(typeErrorName);
results.forEach(
(resultName, result) -> metrics.forEach((
metricName, metric) -> {
String telemetryName = artifactName + resultName + metricName;
TelemetryKey tk = metric.apply(result.apply(artifact));
assertThat(tk.key()).isEqualTo(telemetryName);
}));
});
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.slf4j.event.Level;
import org.sonar.api.SonarEdition;
import org.sonar.api.SonarQubeSide;
import org.sonar.api.batch.fs.FileSystem;
import org.sonar.api.batch.fs.InputFile;
import org.sonar.api.batch.fs.internal.DefaultFileSystem;
import org.sonar.api.batch.fs.internal.DefaultInputFile;
Expand Down Expand Up @@ -123,6 +124,7 @@ void test_issues_creation_on_main_file() throws IOException {
assertThat(telemetryMap).containsOnlyKeys(
"java.analysis.main.success.size_chars",
"java.analysis.main.success.time_ms",
"java.analysis.main.success.type_error_count",
"java.dependency.lombok",
"java.dependency.spring-boot",
"java.dependency.spring-web",
Expand All @@ -132,16 +134,19 @@ void test_issues_creation_on_main_file() throws IOException {
"java.scanner_app");
assertThat(telemetryMap.get("java.analysis.main.success.size_chars")).matches("\\d{5}");
assertThat(telemetryMap.get("java.analysis.main.success.time_ms")).matches("\\d+");
assertThat(telemetryMap).containsEntry("java.analysis.main.success.type_error_count", "199");
}

@Test
// Renaming this method will break lineNumberOfTheMethodWithNoSonar(fs). The name is used to locate the line number.
void test_issues_creation_on_test_file() throws IOException { // NOSONAR required to test NOSONAR reporting on test files
testIssueCreation(InputFile.Type.TEST, 0);

Map<String, String> telemetryMap = telemetry.toMap();
assertThat(telemetryMap).containsOnlyKeys(
"java.analysis.test.success.size_chars",
"java.analysis.test.success.time_ms",
"java.analysis.test.success.type_error_count",
"java.dependency.lombok",
"java.dependency.spring-boot",
"java.dependency.spring-web",
Expand All @@ -151,6 +156,15 @@ void test_issues_creation_on_test_file() throws IOException { // NOSONAR require
"java.scanner_app");
assertThat(telemetryMap.get("java.analysis.test.success.size_chars")).matches("\\d{5}");
assertThat(telemetryMap.get("java.analysis.test.success.time_ms")).matches("\\d+");
assertThat(telemetryMap).containsEntry("java.analysis.test.success.type_error_count", "199");
}

private static int lineNumberOfTheMethodWithNoSonar(FileSystem fs) throws IOException {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for fixing this!

To avoid problems caused by refactoring. let's add the following comment on the test test_issues_creation_on_test_file:

// Renaming this test will break stuff. The name is used to locate the line number.

String[] lines = fs.inputFile(fs.predicates().hasPath("org/sonar/plugins/java/JavaSensorTest.java")).contents().split("\n");
int zeroBasedLineIndex = (int) Stream.of(lines)
.takeWhile(line -> !line.contains("test_issues_creation_on_test_file"))
.count();
return zeroBasedLineIndex + 1;
}

private void testIssueCreation(InputFile.Type onType, int expectedIssues) throws IOException {
Expand All @@ -169,8 +183,8 @@ private void testIssueCreation(InputFile.Type onType, int expectedIssues) throws
JavaSensor jss = new JavaSensor(sonarComponents, fs, javaResourceLocator, settings.asConfig(), noSonarFilter, null, telemetry);

jss.execute(context);
// argument 138 refers to the comment on line #138 in this file, each time this file changes, this argument should be updated
verify(noSonarFilter, times(1)).noSonarInFile(fs.inputFiles().iterator().next(), Collections.singleton(138));
int expectedNoSonarLine = lineNumberOfTheMethodWithNoSonar(fs);
verify(noSonarFilter, times(1)).noSonarInFile(fs.inputFiles().iterator().next(), Collections.singleton(expectedNoSonarLine));
verify(sonarComponents, times(expectedIssues)).reportIssue(any(AnalyzerMessage.class));

// There are additional entries, but we do not test them.
Expand Down