SONARJAVA-5695 Report speed of analysis and analysis errors - #5259
Conversation
c651ac5 to
af509d0
Compare
932a728 to
d7d730b
Compare
|
|
||
| assertThat(extractTelemetryLogs(buildResult)) | ||
| .matches(patternWithLiteralDot(""" | ||
| Telemetry java.analysis.generated.success.size_chars: \\d{5} |
There was a problem hiding this comment.
Let's add a comment that the size of the generated files is not matched exactly, beaucase it varies due depending on the environment.
| private VisitorsBridge visitor; | ||
| private boolean reportedMisconfiguredVersion = false; | ||
|
|
||
| public JavaAstScanner(@Nullable SonarComponents sonarComponents) { |
There was a problem hiding this comment.
It looks like it is used only for tests. Let's remove this constructor and use the prod version. (Or at leaste annotate it @VisibleForTesting)
| return prefix + suffix; | ||
| } | ||
|
|
||
| public static int charCount(InputFile inputFile, int defaultValue) { |
There was a problem hiding this comment.
Let's change defaultValue to onError (or something similar, like valueOnError / defaultOnError)
| try { | ||
| return inputFile.contents().length(); | ||
| } catch (IOException e) { | ||
| LOG.debug("ERROR, failed to get content size for: {}, {}: {}" , inputFile, e.getClass().getSimpleName(), e.getMessage()); |
There was a problem hiding this comment.
Error instead of ERROR becuse the latter is a valid log level.
| @Test | ||
| void char_count() throws IOException { | ||
| InputFile inputFile = Mockito.mock(InputFile.class); | ||
| Mockito.when(inputFile.contents()).thenReturn("1234"); |
There was a problem hiding this comment.
Optional: import static Mockito.when
|
|
||
| @Test | ||
| void char_count() throws IOException { | ||
| InputFile inputFile = Mockito.mock(InputFile.class); |
There was a problem hiding this comment.
Let make inputFile a field, create the mock in @Before (alternative,, use @mock).
(when(inputFile) should stay in @Test)
| "time_ms", SpeedKeys::timeMsKey | ||
| ); | ||
| artifacts.forEach((artifactName, artifact) -> results.forEach( | ||
| (resultName, result) -> metrics.forEach(( |
There was a problem hiding this comment.
Formatting :)
|
|
||
| // There are additional entries, but we do not test them. | ||
| assertThat(telemetry.toMap()).contains( | ||
| String codeType = (onType == InputFile.Type.MAIN) ? "main" : "test"; |
There was a problem hiding this comment.
Let's move the assertion to the each test, so we do not need this logic here.
|
|
||
| } | ||
|
|
||
| public static Map<String, String> normalizeTimeMs(Map<String, String> map) { |
There was a problem hiding this comment.
Let's use more features of assertJ, so we can assert that we have the right keys without checking the values.
48f875a to
aa2a1f5
Compare
…rator from randomly failing with OOMKilled when the in memory tmp filesystem becomes too big
… when, times, verify, ...
aa2a1f5 to
bb5dd8d
Compare
|
| InputFile inputFile = Mockito.mock(InputFile.class); | ||
| Mockito | ||
| .doReturn("/tmp/Example.java") | ||
| InputFile inputFile = mock(InputFile.class); |
There was a problem hiding this comment.
No need to change anything, but I just learned https://stackoverflow.com/questions/20353846/mockito-difference-between-doreturn-and-when and wonder if we should have when(...).thenReturn(...) here. We have a mock and not a spy here, so when/thenReturn and doReturn/when work the same in this case, but the latter is more idiomatic.
2 participants
SONARJAVA-5695