From 516f6e6bd2c8d82ffaac2aa8f73f063618b972c1 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Mon, 10 Sep 2018 14:17:40 +0200 Subject: [PATCH] SONAR-11238 record warning when broken highlighting data in report --- ...ProjectAnalysisTaskContainerPopulator.java | 6 + .../source/FileSourceDataComputer.java | 27 +-- .../source/FileSourceDataWarnings.java | 93 ++++++++ .../source/SourceLineReadersFactory.java | 28 ++- .../source/linereader/CoverageLineReader.java | 4 +- .../linereader/DuplicationLineReader.java | 4 +- .../linereader/HighlightingLineReader.java | 27 ++- .../source/linereader/IsNewLineReader.java | 5 +- .../source/linereader/LineReader.java | 58 ++++- .../source/linereader/ScmLineReader.java | 6 +- .../source/linereader/SymbolsLineReader.java | 31 +-- .../step/PersistFileSourcesStep.java | 10 +- ...ectAnalysisTaskContainerPopulatorTest.java | 54 ++--- .../source/FileSourceDataComputerTest.java | 155 +++++++++---- .../source/FileSourceDataWarningsTest.java | 219 ++++++++++++++++++ .../source/LineReadersImplTest.java | 146 ++++++++++++ .../source/SourceLineReadersFactoryTest.java | 76 +----- .../linereader/CoverageLineReaderTest.java | 10 +- .../linereader/DuplicationLineReaderTest.java | 50 ++-- .../HighlightingLineReaderTest.java | 56 +++-- .../source/linereader/ScmLineReaderTest.java | 6 +- .../linereader/SymbolsLineReaderTest.java | 67 +++--- .../step/PersistFileSourcesStepTest.java | 33 ++- .../org/sonar/ce/task/step/StepsExplorer.java | 68 ------ 24 files changed, 880 insertions(+), 359 deletions(-) create mode 100644 server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/FileSourceDataWarnings.java create mode 100644 server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/FileSourceDataWarningsTest.java create mode 100644 server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/LineReadersImplTest.java delete mode 100644 server/sonar-ce-task/src/test/java/org/sonar/ce/task/step/StepsExplorer.java diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java index 4a0eb5c372f..f3d2d0a1e77 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java @@ -24,6 +24,7 @@ import java.util.List; import javax.annotation.Nullable; import org.sonar.ce.task.CeTask; import org.sonar.ce.task.container.TaskContainer; +import org.sonar.ce.task.log.CeTaskMessagesImpl; import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolderImpl; import org.sonar.ce.task.projectanalysis.api.posttask.PostProjectAnalysisTasksExecutor; import org.sonar.ce.task.projectanalysis.batch.BatchReportDirectoryHolderImpl; @@ -112,6 +113,7 @@ import org.sonar.ce.task.projectanalysis.scm.ScmInfoDbLoader; import org.sonar.ce.task.projectanalysis.scm.ScmInfoRepositoryImpl; import org.sonar.ce.task.projectanalysis.source.DbLineHashVersion; import org.sonar.ce.task.projectanalysis.source.FileSourceDataComputer; +import org.sonar.ce.task.projectanalysis.source.FileSourceDataWarnings; import org.sonar.ce.task.projectanalysis.source.LastCommitVisitor; import org.sonar.ce.task.projectanalysis.source.NewLinesRepository; import org.sonar.ce.task.projectanalysis.source.SignificantCodeRepository; @@ -166,6 +168,10 @@ public final class ProjectAnalysisTaskContainerPopulator implements ContainerPop PostProjectAnalysisTasksExecutor.class, ComputationStepExecutor.class, + // messages/warnings + CeTaskMessagesImpl.class, + FileSourceDataWarnings.class, + // File System new ComputationTempFolderProvider(), diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/FileSourceDataComputer.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/FileSourceDataComputer.java index b560a23f239..13de8bf00d4 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/FileSourceDataComputer.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/FileSourceDataComputer.java @@ -42,7 +42,7 @@ public class FileSourceDataComputer { this.sourceHashComputer = new SourceHashComputer(); } - public Data compute(Component file) { + public Data compute(Component file, FileSourceDataWarnings fileSourceDataWarnings) { try (CloseableIterator linesIterator = sourceLinesRepository.readLines(file); SourceLineReadersFactory.LineReaders lineReaders = sourceLineReadersFactory.getLineReaders(file)) { SourceLinesHashRepositoryImpl.LineHashesComputer lineHashesComputer = sourceLinesHash.getLineHashesComputerToPersist(file); @@ -51,7 +51,17 @@ public class FileSourceDataComputer { while (linesIterator.hasNext()) { currentLine++; - read(fileSourceBuilder, lineHashesComputer, lineReaders, currentLine, linesIterator.next(), linesIterator.hasNext()); + String lineSource = linesIterator.next(); + boolean hasNextLine = linesIterator.hasNext(); + + sourceHashComputer.addLine(lineSource, hasNextLine); + lineHashesComputer.addLine(lineSource); + + DbFileSources.Line.Builder lineBuilder = fileSourceBuilder + .addLinesBuilder() + .setSource(lineSource) + .setLine(currentLine); + lineReaders.read(lineBuilder, readError -> fileSourceDataWarnings.addWarning(file, readError)); } Changeset latestChangeWithRevision = lineReaders.getLatestChangeWithRevision(); @@ -59,19 +69,6 @@ public class FileSourceDataComputer { } } - private void read(DbFileSources.Data.Builder fileSourceBuilder, SourceLinesHashRepositoryImpl.LineHashesComputer lineHashesComputer, - SourceLineReadersFactory.LineReaders lineReaders, int currentLine, String lineSource, boolean hasNextLine) { - sourceHashComputer.addLine(lineSource, hasNextLine); - lineHashesComputer.addLine(lineSource); - - DbFileSources.Line.Builder lineBuilder = fileSourceBuilder - .addLinesBuilder() - .setSource(lineSource) - .setLine(currentLine); - - lineReaders.read(lineBuilder); - } - public static class Data { private final DbFileSources.Data fileSourceData; private final List lineHashes; diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/FileSourceDataWarnings.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/FileSourceDataWarnings.java new file mode 100644 index 00000000000..f43770e9ff5 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/FileSourceDataWarnings.java @@ -0,0 +1,93 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.ce.task.projectanalysis.source; + +import java.util.Comparator; +import java.util.EnumMap; +import java.util.HashSet; +import java.util.Set; +import org.sonar.api.utils.System2; +import org.sonar.ce.task.log.CeTaskMessages; +import org.sonar.ce.task.projectanalysis.component.Component; +import org.sonar.ce.task.projectanalysis.source.linereader.LineReader; + +import static com.google.common.base.Preconditions.checkState; +import static java.lang.String.format; +import static java.util.Objects.requireNonNull; +import static java.util.stream.Collectors.joining; + +public class FileSourceDataWarnings { + private static final Comparator COMPONENT_COMPARATOR = Comparator.comparingInt(t -> t.getReportAttributes().getRef()); + + private final CeTaskMessages taskMessages; + private final System2 system2; + private final EnumMap> fileErrorsPerData = new EnumMap<>(LineReader.Data.class); + private boolean closed = false; + + public FileSourceDataWarnings(CeTaskMessages taskMessages, System2 system2) { + this.taskMessages = taskMessages; + this.system2 = system2; + } + + public void addWarning(Component file, LineReader.ReadError readError) { + checkNotCommitted(); + requireNonNull(file, "file can't be null"); + requireNonNull(readError, "readError can't be null"); + + fileErrorsPerData.compute(readError.getData(), (data, existingList) -> { + Set res = existingList == null ? new HashSet<>() : existingList; + res.add(file); + return res; + }); + } + + public void commitWarnings() { + checkNotCommitted(); + this.closed = true; + Set filesWithHighlightingErrors = fileErrorsPerData.get(LineReader.Data.HIGHLIGHTING); + if (filesWithHighlightingErrors == null) { + return; + } + + taskMessages.add(new CeTaskMessages.Message(computeMessage(filesWithHighlightingErrors), system2.now())); + } + + private static String computeMessage(Set filesWithHighlightingErrors) { + if (filesWithHighlightingErrors.size() == 1) { + Component file = filesWithHighlightingErrors.iterator().next(); + return format("Inconsistent highlighting data detected on file '%s'. " + + "File source may have been modified while analysis was running.", + file.getReportAttributes().getPath()); + } + String lineHeader = "\n ° "; + return format("Inconsistent highlighting data detected on some files (%s in total). " + + "File source may have been modified while analysis was running.", filesWithHighlightingErrors.size()) + + filesWithHighlightingErrors.stream() + .sorted(COMPONENT_COMPARATOR) + .limit(5) + .map(component -> component.getReportAttributes().getPath()) + .collect(joining(lineHeader, lineHeader, "")); + } + + private void checkNotCommitted() { + checkState(!closed, "warnings already commit"); + } + +} diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/SourceLineReadersFactory.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/SourceLineReadersFactory.java index eb22f1ca4cb..1308b263a88 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/SourceLineReadersFactory.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/SourceLineReadersFactory.java @@ -19,9 +19,11 @@ */ package org.sonar.ce.task.projectanalysis.source; +import com.google.common.annotations.VisibleForTesting; import java.util.ArrayList; import java.util.List; import java.util.Optional; +import java.util.function.Consumer; import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.sonar.ce.task.projectanalysis.batch.BatchReportReader; @@ -83,33 +85,47 @@ public class SourceLineReadersFactory { readers.add(new DuplicationLineReader(duplicationRepository.getDuplications(component))); readers.add(new IsNewLineReader(newLinesRepository, component)); - return new LineReaders(readers, scmLineReader, closeables); + return new LineReadersImpl(readers, scmLineReader, closeables); } - static class LineReaders implements AutoCloseable, LineReader { + interface LineReaders extends AutoCloseable { + void read(DbFileSources.Line.Builder lineBuilder, Consumer readErrorConsumer); + + @CheckForNull + Changeset getLatestChangeWithRevision(); + + @Override + void close(); + } + + @VisibleForTesting + static final class LineReadersImpl implements LineReaders { final List readers; @Nullable final ScmLineReader scmLineReader; final List> closeables; - LineReaders(List readers, @Nullable ScmLineReader scmLineReader, List> closeables) { + LineReadersImpl(List readers, @Nullable ScmLineReader scmLineReader, List> closeables) { this.readers = readers; this.scmLineReader = scmLineReader; this.closeables = closeables; } - @Override public void close() { + @Override + public void close() { for (CloseableIterator reportIterator : closeables) { reportIterator.close(); } } - @Override public void read(DbFileSources.Line.Builder lineBuilder) { + public void read(DbFileSources.Line.Builder lineBuilder, Consumer readErrorConsumer) { for (LineReader r : readers) { - r.read(lineBuilder); + r.read(lineBuilder) + .ifPresent(readErrorConsumer); } } + @Override @CheckForNull public Changeset getLatestChangeWithRevision() { return scmLineReader == null ? null : scmLineReader.getLatestChangeWithRevision(); diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/CoverageLineReader.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/CoverageLineReader.java index 43540c7e058..4967f62fac1 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/CoverageLineReader.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/CoverageLineReader.java @@ -20,6 +20,7 @@ package org.sonar.ce.task.projectanalysis.source.linereader; import java.util.Iterator; +import java.util.Optional; import javax.annotation.CheckForNull; import org.sonar.db.protobuf.DbFileSources; import org.sonar.scanner.protocol.output.ScannerReport; @@ -36,12 +37,13 @@ public class CoverageLineReader implements LineReader { } @Override - public void read(DbFileSources.Line.Builder lineBuilder) { + public Optional read(DbFileSources.Line.Builder lineBuilder) { ScannerReport.LineCoverage reportCoverage = getNextLineCoverageIfMatchLine(lineBuilder.getLine()); if (reportCoverage != null) { processCoverage(lineBuilder, reportCoverage); coverage = null; } + return Optional.empty(); } private static void processCoverage(DbFileSources.Line.Builder lineBuilder, ScannerReport.LineCoverage reportCoverage) { diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/DuplicationLineReader.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/DuplicationLineReader.java index 6b885074ef0..4e0184134ef 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/DuplicationLineReader.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/DuplicationLineReader.java @@ -26,6 +26,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Optional; import javax.annotation.Nonnull; import javax.annotation.Nullable; import org.sonar.ce.task.projectanalysis.duplication.Duplicate; @@ -46,7 +47,7 @@ public class DuplicationLineReader implements LineReader { } @Override - public void read(DbFileSources.Line.Builder lineBuilder) { + public Optional read(DbFileSources.Line.Builder lineBuilder) { Predicate> containsLine = new TextBlockContainsLine(lineBuilder.getLine()); for (Integer textBlockIndex : from(duplicatedTextBlockIndexByTextBlock.entrySet()) .filter(containsLine) @@ -56,6 +57,7 @@ public class DuplicationLineReader implements LineReader { .toSortedList(Ordering.natural())) { lineBuilder.addDuplication(textBlockIndex); } + return Optional.empty(); } /** diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/HighlightingLineReader.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/HighlightingLineReader.java index 8a583971707..73668d40c5f 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/HighlightingLineReader.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/HighlightingLineReader.java @@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableMap; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Optional; import javax.annotation.CheckForNull; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; @@ -34,6 +35,7 @@ import org.sonar.scanner.protocol.output.ScannerReport.SyntaxHighlightingRule.Hi import static com.google.common.collect.Lists.newArrayList; import static java.lang.String.format; +import static org.sonar.ce.task.projectanalysis.source.linereader.LineReader.Data.HIGHLIGHTING; import static org.sonar.ce.task.projectanalysis.source.linereader.RangeOffsetConverter.OFFSET_SEPARATOR; import static org.sonar.ce.task.projectanalysis.source.linereader.RangeOffsetConverter.SYMBOLS_SEPARATOR; @@ -41,7 +43,7 @@ public class HighlightingLineReader implements LineReader { private static final Logger LOG = Loggers.get(HighlightingLineReader.class); - private boolean isHighlightingValid = true; + private ReadError readError = null; private static final Map cssClassByType = ImmutableMap.builder() .put(HighlightingType.ANNOTATION, "a") @@ -69,17 +71,22 @@ public class HighlightingLineReader implements LineReader { this.highlightingList = newArrayList(); } + /** + * Stops reading at first encountered error, which implies the same + * {@link org.sonar.ce.task.projectanalysis.source.linereader.LineReader.ReadError} will be returned once an error + * has been encountered and for any then provided {@link DbFileSources.Line.Builder lineBuilder}. + */ @Override - public void read(DbFileSources.Line.Builder lineBuilder) { - if (!isHighlightingValid) { - return; - } - try { - processHighlightings(lineBuilder); - } catch (RangeOffsetConverterException e) { - isHighlightingValid = false; - LOG.warn(format("Inconsistency detected in Highlighting data. Highlighting will be ignored for file '%s'", file.getDbKey()), e); + public Optional read(DbFileSources.Line.Builder lineBuilder) { + if (readError == null) { + try { + processHighlightings(lineBuilder); + } catch (RangeOffsetConverterException e) { + readError = new ReadError(HIGHLIGHTING, lineBuilder.getLine()); + LOG.warn(format("Inconsistency detected in Highlighting data. Highlighting will be ignored for file '%s'", file.getDbKey()), e); + } } + return Optional.ofNullable(readError); } private void processHighlightings(DbFileSources.Line.Builder lineBuilder) { diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/IsNewLineReader.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/IsNewLineReader.java index f4d158c84ae..e70d0a09fd7 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/IsNewLineReader.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/IsNewLineReader.java @@ -20,6 +20,7 @@ package org.sonar.ce.task.projectanalysis.source.linereader; import java.util.Collections; +import java.util.Optional; import java.util.Set; import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.ce.task.projectanalysis.source.NewLinesRepository; @@ -32,7 +33,9 @@ public class IsNewLineReader implements LineReader { this.newLines = newLinesRepository.getNewLines(file).orElse(Collections.emptySet()); } - @Override public void read(DbFileSources.Line.Builder lineBuilder) { + @Override + public Optional read(DbFileSources.Line.Builder lineBuilder) { lineBuilder.setIsNewLine(newLines.contains(lineBuilder.getLine())); + return Optional.empty(); } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/LineReader.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/LineReader.java index fa1a4153e02..70b1f1f6a2c 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/LineReader.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/LineReader.java @@ -19,11 +19,67 @@ */ package org.sonar.ce.task.projectanalysis.source.linereader; +import java.util.Objects; +import java.util.Optional; +import javax.annotation.concurrent.Immutable; import org.sonar.db.protobuf.DbFileSources; +import static com.google.common.base.Preconditions.checkArgument; +import static java.util.Objects.requireNonNull; + @FunctionalInterface public interface LineReader { - void read(DbFileSources.Line.Builder lineBuilder); + Optional read(DbFileSources.Line.Builder lineBuilder); + + enum Data { + COVERAGE, DUPLICATION, HIGHLIGHTING, SCM, SYMBOLS + } + + @Immutable + final class ReadError { + private final Data data; + private final int line; + + public ReadError(Data data, int line) { + requireNonNull(data); + checkArgument(line >= 0); + this.data = data; + this.line = line; + } + + public Data getData() { + return data; + } + + public int getLine() { + return line; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + ReadError readError = (ReadError) o; + return line == readError.line && + data == readError.data; + } + + @Override + public int hashCode() { + return Objects.hash(data, line); + } + @Override + public String toString() { + return "ReadError{" + + "data=" + data + + ", line=" + line + + '}'; + } + } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/ScmLineReader.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/ScmLineReader.java index da1fd077cc3..181d2b054b3 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/ScmLineReader.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/ScmLineReader.java @@ -19,10 +19,11 @@ */ package org.sonar.ce.task.projectanalysis.source.linereader; +import java.util.Optional; import javax.annotation.CheckForNull; -import org.sonar.db.protobuf.DbFileSources; import org.sonar.ce.task.projectanalysis.scm.Changeset; import org.sonar.ce.task.projectanalysis.scm.ScmInfo; +import org.sonar.db.protobuf.DbFileSources; public class ScmLineReader implements LineReader { @@ -37,7 +38,7 @@ public class ScmLineReader implements LineReader { } @Override - public void read(DbFileSources.Line.Builder lineBuilder) { + public Optional read(DbFileSources.Line.Builder lineBuilder) { if (scmReport.hasChangesetForLine(lineBuilder.getLine())) { Changeset changeset = scmReport.getChangesetForLine(lineBuilder.getLine()); String author = changeset.getAuthor(); @@ -55,6 +56,7 @@ public class ScmLineReader implements LineReader { updateLatestChangeWithRevision(changeset); } } + return Optional.empty(); } private void updateLatestChange(Changeset newChangeSet) { diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/SymbolsLineReader.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/SymbolsLineReader.java index 1b75dbb3c53..7953b5360d8 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/SymbolsLineReader.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/source/linereader/SymbolsLineReader.java @@ -23,12 +23,12 @@ import com.google.common.collect.HashMultimap; import com.google.common.collect.Lists; import com.google.common.collect.SetMultimap; import java.util.ArrayList; -import java.util.Collections; import java.util.Comparator; import java.util.IdentityHashMap; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Optional; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; import org.sonar.ce.task.projectanalysis.component.Component; @@ -48,30 +48,35 @@ public class SymbolsLineReader implements LineReader { private final Map idsBySymbol; private final SetMultimap symbolsPerLine; - private boolean areSymbolsValid = true; + private ReadError readError = null; public SymbolsLineReader(Component file, Iterator symbolIterator, RangeOffsetConverter rangeOffsetConverter) { this.file = file; this.rangeOffsetConverter = rangeOffsetConverter; List symbols = Lists.newArrayList(symbolIterator); // Sort symbols to have deterministic id generation - Collections.sort(symbols, SymbolsComparator.INSTANCE); + symbols.sort(SymbolsComparator.INSTANCE); this.idsBySymbol = createIdsBySymbolMap(symbols); this.symbolsPerLine = buildSymbolsPerLine(symbols); } + /** + * Stops reading at first encountered error, which implies the same + * {@link org.sonar.ce.task.projectanalysis.source.linereader.LineReader.ReadError} will be returned once an error + * has been encountered and for any then provided {@link DbFileSources.Line.Builder lineBuilder}. + */ @Override - public void read(DbFileSources.Line.Builder lineBuilder) { - if (!areSymbolsValid) { - return; - } - try { - processSymbols(lineBuilder); - } catch (RangeOffsetConverter.RangeOffsetConverterException e) { - areSymbolsValid = false; - LOG.warn(format("Inconsistency detected in Symbols data. Symbols will be ignored for file '%s'", file.getDbKey()), e); + public Optional read(DbFileSources.Line.Builder lineBuilder) { + if (readError == null) { + try { + processSymbols(lineBuilder); + } catch (RangeOffsetConverter.RangeOffsetConverterException e) { + readError = new ReadError(Data.SYMBOLS, lineBuilder.getLine()); + LOG.warn(format("Inconsistency detected in Symbols data. Symbols will be ignored for file '%s'", file.getDbKey()), e); + } } + return Optional.ofNullable(readError); } private void processSymbols(DbFileSources.Line.Builder lineBuilder) { @@ -80,7 +85,7 @@ public class SymbolsLineReader implements LineReader { List lineSymbols = new ArrayList<>(this.symbolsPerLine.get(line)); // Sort symbols to have deterministic results and avoid false variation that would lead to an unnecessary update of the source files // data - Collections.sort(lineSymbols, SymbolsComparator.INSTANCE); + lineSymbols.sort(SymbolsComparator.INSTANCE); StringBuilder symbolString = new StringBuilder(); for (ScannerReport.Symbol lineSymbol : lineSymbols) { diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistFileSourcesStep.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistFileSourcesStep.java index c8deeaed8b6..5931994dd01 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistFileSourcesStep.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistFileSourcesStep.java @@ -35,6 +35,7 @@ import org.sonar.ce.task.projectanalysis.component.TreeRootHolder; import org.sonar.ce.task.projectanalysis.component.TypeAwareVisitorAdapter; import org.sonar.ce.task.projectanalysis.scm.Changeset; import org.sonar.ce.task.projectanalysis.source.FileSourceDataComputer; +import org.sonar.ce.task.projectanalysis.source.FileSourceDataWarnings; import org.sonar.ce.task.projectanalysis.source.SourceLinesHashRepository; import org.sonar.ce.task.step.ComputationStep; import org.sonar.db.DbClient; @@ -51,14 +52,17 @@ public class PersistFileSourcesStep implements ComputationStep { private final TreeRootHolder treeRootHolder; private final SourceLinesHashRepository sourceLinesHash; private final FileSourceDataComputer fileSourceDataComputer; + private final FileSourceDataWarnings fileSourceDataWarnings; public PersistFileSourcesStep(DbClient dbClient, System2 system2, TreeRootHolder treeRootHolder, - SourceLinesHashRepository sourceLinesHash, FileSourceDataComputer fileSourceDataComputer) { + SourceLinesHashRepository sourceLinesHash, FileSourceDataComputer fileSourceDataComputer, + FileSourceDataWarnings fileSourceDataWarnings) { this.dbClient = dbClient; this.system2 = system2; this.treeRootHolder = treeRootHolder; this.sourceLinesHash = sourceLinesHash; this.fileSourceDataComputer = fileSourceDataComputer; + this.fileSourceDataWarnings = fileSourceDataWarnings; } @Override @@ -67,6 +71,8 @@ public class PersistFileSourcesStep implements ComputationStep { try (DbSession dbSession = dbClient.openSession(false)) { new DepthTraversalTypeAwareCrawler(new FileSourceVisitor(dbSession)) .visit(treeRootHolder.getRoot()); + } finally { + fileSourceDataWarnings.commitWarnings(); } } @@ -94,7 +100,7 @@ public class PersistFileSourcesStep implements ComputationStep { @Override public void visitFile(Component file) { try { - FileSourceDataComputer.Data fileSourceData = fileSourceDataComputer.compute(file); + FileSourceDataComputer.Data fileSourceData = fileSourceDataComputer.compute(file, fileSourceDataWarnings); persistSource(fileSourceData, file); } catch (Exception e) { throw new IllegalStateException(String.format("Cannot persist sources of %s", file.getDbKey()), e); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulatorTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulatorTest.java index 2cb8dc10a5b..b8e429a700f 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulatorTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulatorTest.java @@ -22,23 +22,25 @@ package org.sonar.ce.task.projectanalysis.container; import com.google.common.base.Function; import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; +import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Objects; import java.util.Set; +import java.util.stream.Collectors; import javax.annotation.Nonnull; -import javax.annotation.Nullable; import org.junit.Test; import org.picocontainer.DefaultPicoContainer; import org.picocontainer.PicoContainer; +import org.reflections.Reflections; import org.sonar.ce.task.CeTask; import org.sonar.ce.task.container.TaskContainer; import org.sonar.ce.task.projectanalysis.step.PersistComponentsStep; import org.sonar.ce.task.step.ComputationStep; -import org.sonar.ce.task.step.StepsExplorer; import org.sonar.core.platform.ComponentContainer; +import org.sonar.core.util.stream.MoreCollectors; -import static com.google.common.base.Predicates.notNull; import static com.google.common.collect.FluentIterable.from; import static com.google.common.collect.Sets.difference; import static org.assertj.core.api.Assertions.assertThat; @@ -66,23 +68,32 @@ public class ProjectAnalysisTaskContainerPopulatorTest { AddedObjectsRecorderTaskContainer container = new AddedObjectsRecorderTaskContainer(); underTest.populateContainer(container); - Set computationStepClassNames = from(container.added) - .transform(new Function>() { - @Nullable - @Override - public Class apply(Object input) { - if (input instanceof Class) { - return (Class) input; - } - return null; + Set computationStepClassNames = container.added.stream() + .map(s -> { + if (s instanceof Class) { + return (Class) s; } + return null; }) - .filter(notNull()) - .filter(IsComputationStep.INSTANCE) - .transform(StepsExplorer.toCanonicalName()) - .toSet(); + .filter(Objects::nonNull) + .filter(ComputationStep.class::isAssignableFrom) + .map(Class::getCanonicalName) + .collect(MoreCollectors.toSet()); + + assertThat(difference(retrieveStepPackageStepsCanonicalNames(PROJECTANALYSIS_STEP_PACKAGE), computationStepClassNames)).isEmpty(); + } - assertThat(difference(StepsExplorer.retrieveStepPackageStepsCanonicalNames(PROJECTANALYSIS_STEP_PACKAGE), computationStepClassNames)).isEmpty(); + /** + * Compute set of canonical names of classes implementing ComputationStep in the specified package using reflection. + */ + private static Set retrieveStepPackageStepsCanonicalNames(String packageName) { + Reflections reflections = new Reflections(packageName); + + return reflections.getSubTypesOf(ComputationStep.class).stream() + .filter(input -> !Modifier.isAbstract(input.getModifiers())) + .map(Class::getCanonicalName) + .filter(Objects::nonNull) + .collect(Collectors.toSet()); } @Test @@ -113,15 +124,6 @@ public class ProjectAnalysisTaskContainerPopulatorTest { } - private enum IsComputationStep implements Predicate> { - INSTANCE; - - @Override - public boolean apply(Class input) { - return ComputationStep.class.isAssignableFrom(input); - } - } - private static class AddedObjectsRecorderTaskContainer implements TaskContainer { private static final DefaultPicoContainer SOME_EMPTY_PICO_CONTAINER = new DefaultPicoContainer(); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/FileSourceDataComputerTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/FileSourceDataComputerTest.java index 7e052242fbd..9554416dd30 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/FileSourceDataComputerTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/FileSourceDataComputerTest.java @@ -19,22 +19,34 @@ */ package org.sonar.ce.task.projectanalysis.source; -import java.util.Arrays; -import java.util.Collections; +import java.util.Iterator; import java.util.List; +import java.util.Random; +import java.util.function.Consumer; +import java.util.stream.IntStream; import org.junit.Before; import org.junit.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.ce.task.projectanalysis.component.ReportComponent; +import org.sonar.ce.task.projectanalysis.scm.Changeset; import org.sonar.ce.task.projectanalysis.source.SourceLinesHashRepositoryImpl.LineHashesComputer; import org.sonar.ce.task.projectanalysis.source.linereader.LineReader; -import org.sonar.ce.task.projectanalysis.source.linereader.ScmLineReader; +import org.sonar.core.hash.SourceHashComputer; import org.sonar.core.util.CloseableIterator; +import org.sonar.db.protobuf.DbFileSources; +import static java.util.stream.Collectors.toList; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.same; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; public class FileSourceDataComputerTest { @@ -44,60 +56,109 @@ public class FileSourceDataComputerTest { private LineHashesComputer lineHashesComputer = mock(LineHashesComputer.class); private SourceLineReadersFactory sourceLineReadersFactory = mock(SourceLineReadersFactory.class); private SourceLinesHashRepository sourceLinesHashRepository = mock(SourceLinesHashRepository.class); - private ScmLineReader scmLineReader = mock(ScmLineReader.class); - private CloseableIterator closeableIterator = mock(CloseableIterator.class); - private FileSourceDataComputer fileSourceDataComputer; + private FileSourceDataComputer underTest = new FileSourceDataComputer(sourceLinesRepository, sourceLineReadersFactory, sourceLinesHashRepository); + + private FileSourceDataWarnings fileSourceDataWarnings = mock(FileSourceDataWarnings.class); + private SourceLineReadersFactory.LineReaders lineReaders = mock(SourceLineReadersFactory.LineReaders.class); @Before public void before() { when(sourceLinesHashRepository.getLineHashesComputerToPersist(FILE)).thenReturn(lineHashesComputer); - LineReader reader = line -> line.setHighlighting("h-" + line.getLine()); - SourceLineReadersFactory.LineReaders lineReaders = new SourceLineReadersFactory.LineReaders(Collections.singletonList(reader), scmLineReader, - Collections.singletonList(closeableIterator)); - when(sourceLineReadersFactory.getLineReaders(FILE)).thenReturn(lineReaders); - - fileSourceDataComputer = new FileSourceDataComputer(sourceLinesRepository, sourceLineReadersFactory, sourceLinesHashRepository); + when(sourceLineReadersFactory.getLineReaders(FILE)).thenReturn(mock(SourceLineReadersFactory.LineReaders.class)); } @Test - public void compute_one_line() { - List lineHashes = Collections.singletonList("lineHash"); - when(sourceLinesRepository.readLines(FILE)).thenReturn(CloseableIterator.from(Collections.singletonList("line1").iterator())); - when(lineHashesComputer.getResult()).thenReturn(lineHashes); - - FileSourceDataComputer.Data data = fileSourceDataComputer.compute(FILE); - - assertThat(data.getLineHashes()).isEqualTo(lineHashes); - assertThat(data.getSrcHash()).isEqualTo("137f72c3708c6bd0de00a0e5a69c699b"); - assertThat(data.getLineData().getLinesList()).hasSize(1); - assertThat(data.getLineData().getLines(0).getHighlighting()).isEqualTo("h-1"); - - verify(lineHashesComputer).addLine("line1"); - verify(lineHashesComputer).getResult(); - verify(closeableIterator).close(); - verifyNoMoreInteractions(lineHashesComputer); + public void compute_calls_read_for_each_line_and_passe_read_error_to_fileSourceDataWarnings() { + int lineCount = 1 + new Random().nextInt(10); + List lines = IntStream.range(0, lineCount).mapToObj(i -> "line" + i).collect(toList()); + when(sourceLinesRepository.readLines(FILE)).thenReturn(CloseableIterator.from(lines.iterator())); + when(sourceLineReadersFactory.getLineReaders(FILE)).thenReturn(lineReaders); + when(sourceLinesHashRepository.getLineHashesComputerToPersist(FILE)).thenReturn(lineHashesComputer); + // mock an implementation that will call the ReadErrorConsumer in order to verify that the provided consumer is + // doing what we expect: pass readError to fileSourceDataWarnings + int randomStartPoint = new Random().nextInt(500); + doAnswer(new Answer() { + int i = randomStartPoint; + + @Override + public Object answer(InvocationOnMock invocation) { + Consumer readErrorConsumer = invocation.getArgument(1); + readErrorConsumer.accept(new LineReader.ReadError(LineReader.Data.SYMBOLS, i++)); + return null; + } + }).when(lineReaders).read(any(), any()); + + underTest.compute(FILE, fileSourceDataWarnings); + + ArgumentCaptor lineBuilderCaptor = ArgumentCaptor.forClass(DbFileSources.Line.Builder.class); + verify(lineReaders, times(lineCount)).read(lineBuilderCaptor.capture(), any()); + assertThat(lineBuilderCaptor.getAllValues()) + .extracting(DbFileSources.Line.Builder::getSource) + .containsOnlyElementsOf(lines); + assertThat(lineBuilderCaptor.getAllValues()) + .extracting(DbFileSources.Line.Builder::getLine) + .containsExactly(IntStream.range(1, lineCount + 1).boxed().toArray(Integer[]::new)); + ArgumentCaptor readErrorCaptor = ArgumentCaptor.forClass(LineReader.ReadError.class); + verify(fileSourceDataWarnings, times(lineCount)).addWarning(same(FILE), readErrorCaptor.capture()); + assertThat(readErrorCaptor.getAllValues()) + .extracting(LineReader.ReadError::getLine) + .containsExactly(IntStream.range(randomStartPoint, randomStartPoint + lineCount).boxed().toArray(Integer[]::new)); } @Test - public void compute_two_lines() { - List lineHashes = Arrays.asList("137f72c3708c6bd0de00a0e5a69c699b", "e6251bcf1a7dc3ba5e7933e325bbe605"); - when(sourceLinesRepository.readLines(FILE)).thenReturn(CloseableIterator.from(Arrays.asList("line1", "line2").iterator())); - when(lineHashesComputer.getResult()).thenReturn(lineHashes); - - FileSourceDataComputer.Data data = fileSourceDataComputer.compute(FILE); - - assertThat(data.getLineHashes()).isEqualTo(lineHashes); - assertThat(data.getSrcHash()).isEqualTo("ee5a58024a155466b43bc559d953e018"); - assertThat(data.getLineData().getLinesList()).hasSize(2); - assertThat(data.getLineData().getLines(0).getHighlighting()).isEqualTo("h-1"); - assertThat(data.getLineData().getLines(1).getHighlighting()).isEqualTo("h-2"); - - verify(lineHashesComputer).addLine("line1"); - verify(lineHashesComputer).addLine("line2"); - verify(lineHashesComputer).getResult(); - verify(closeableIterator).close(); - verifyNoMoreInteractions(lineHashesComputer); + public void compute_builds_data_object_from_lines() { + int lineCount = 1 + new Random().nextInt(10); + int randomStartPoint = new Random().nextInt(500); + List lines = IntStream.range(0, lineCount).mapToObj(i -> "line" + i).collect(toList()); + List expectedLineHashes = IntStream.range(0, 1 + new Random().nextInt(12)).mapToObj(i -> "str_" + i).collect(toList()); + Changeset expectedChangeset = Changeset.newChangesetBuilder().setDate((long) new Random().nextInt(9_999)).build(); + String expectedSrcHash = computeSrcHash(lines); + CloseableIterator lineIterator = spy(CloseableIterator.from(lines.iterator())); + DbFileSources.Data.Builder expectedLineDataBuilder = DbFileSources.Data.newBuilder(); + for (int i = 0; i < lines.size(); i++) { + expectedLineDataBuilder.addLinesBuilder() + .setSource(lines.get(i)) + .setLine(i + 1) + // scmAuthor will be set with specific value by our mock implementation of LinesReaders.read() + .setScmAuthor("reader_called_" + (randomStartPoint + i)); + } + when(sourceLinesRepository.readLines(FILE)).thenReturn(lineIterator); + when(sourceLineReadersFactory.getLineReaders(FILE)).thenReturn(lineReaders); + when(sourceLinesHashRepository.getLineHashesComputerToPersist(FILE)).thenReturn(lineHashesComputer); + when(lineHashesComputer.getResult()).thenReturn(expectedLineHashes); + when(lineReaders.getLatestChangeWithRevision()).thenReturn(expectedChangeset); + // mocked implementation of LineReader.read to ensure changes done by it to the lineBuilder argument actually end + // up in the FileSourceDataComputer.Data object returned + doAnswer(new Answer() { + int i = 0; + + @Override + public Object answer(InvocationOnMock invocation) { + DbFileSources.Line.Builder lineBuilder = invocation.getArgument(0); + lineBuilder.setScmAuthor("reader_called_" + (randomStartPoint + i++)); + return null; + } + }).when(lineReaders).read(any(), any()); + + FileSourceDataComputer.Data data = underTest.compute(FILE, fileSourceDataWarnings); + + assertThat(data.getLineHashes()).isEqualTo(expectedLineHashes); + assertThat(data.getSrcHash()).isEqualTo(expectedSrcHash); + assertThat(data.getLatestChangeWithRevision()).isSameAs(expectedChangeset); + assertThat(data.getLineData()).isEqualTo(expectedLineDataBuilder.build()); + + verify(lineIterator).close(); + verify(lineReaders).close(); + } + + private static String computeSrcHash(List lines) { + SourceHashComputer computer = new SourceHashComputer(); + Iterator iterator = lines.iterator(); + while (iterator.hasNext()) { + computer.addLine(iterator.next(), iterator.hasNext()); + } + return computer.getHash(); } } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/FileSourceDataWarningsTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/FileSourceDataWarningsTest.java new file mode 100644 index 00000000000..221cdf6013e --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/FileSourceDataWarningsTest.java @@ -0,0 +1,219 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.ce.task.projectanalysis.source; + +import com.tngtech.java.junit.dataprovider.DataProvider; +import com.tngtech.java.junit.dataprovider.DataProviderRunner; +import com.tngtech.java.junit.dataprovider.UseDataProvider; +import java.util.Arrays; +import java.util.Random; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.junit.runner.RunWith; +import org.sonar.api.utils.System2; +import org.sonar.ce.task.log.CeTaskMessages; +import org.sonar.ce.task.projectanalysis.component.Component; +import org.sonar.ce.task.projectanalysis.component.ReportComponent; +import org.sonar.ce.task.projectanalysis.source.linereader.LineReader; + +import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; +import static org.sonar.ce.task.projectanalysis.source.linereader.LineReader.Data.HIGHLIGHTING; + +@RunWith(DataProviderRunner.class) +public class FileSourceDataWarningsTest { + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + private CeTaskMessages taskMessages = mock(CeTaskMessages.class); + private System2 system2 = mock(System2.class); + private Random random = new Random(); + private int line = 1 + new Random().nextInt(200); + private long timeStamp = 9_887L + new Random().nextInt(300); + private String path = randomAlphabetic(50); + + private FileSourceDataWarnings underTest = new FileSourceDataWarnings(taskMessages, system2); + + @Test + public void addWarning_fails_with_NPE_if_file_is_null() { + LineReader.ReadError readError = new LineReader.ReadError(HIGHLIGHTING, 2); + + expectedException.expect(NullPointerException.class); + expectedException.expectMessage("file can't be null"); + + underTest.addWarning(null, readError); + } + + @Test + public void addWarning_fails_with_NPE_if_readError_is_null() { + Component component = mock(Component.class); + + expectedException.expect(NullPointerException.class); + expectedException.expectMessage("readError can't be null"); + + underTest.addWarning(component, null); + } + + @Test + public void addWarnings_fails_with_ISE_if_called_after_commitWarnings() { + underTest.commitWarnings(); + + expectedException.expect(IllegalStateException.class); + expectedException.expectMessage("warnings already commit"); + + underTest.addWarning(null /*doesn't matter*/, null /*doesn't matter*/); + } + + @Test + public void commitWarnings_fails_with_ISE_if_called_after_commitWarnings() { + underTest.commitWarnings(); + + expectedException.expect(IllegalStateException.class); + expectedException.expectMessage("warnings already commit"); + + underTest.commitWarnings(); + } + + @Test + public void create_highlighting_warning_when_one_file_HIGHLIGHT_read_error() { + ReportComponent file = ReportComponent.builder(Component.Type.FILE, 1) + .setUuid("uuid") + .setPath(path) + .build(); + LineReader.ReadError readError = new LineReader.ReadError(HIGHLIGHTING, line); + when(system2.now()).thenReturn(timeStamp); + + underTest.addWarning(file, readError); + + verifyZeroInteractions(taskMessages); + + underTest.commitWarnings(); + + verify(taskMessages, times(1)) + .add(new CeTaskMessages.Message("Inconsistent highlighting data detected on file '" + path + "'. " + + "File source may have been modified while analysis was running.", timeStamp)); + } + + @Test + public void create_highlighting_warning_when_any_number_of_read_error_for_one_file() { + ReportComponent file = ReportComponent.builder(Component.Type.FILE, 1) + .setUuid("uuid") + .setPath(path) + .build(); + LineReader.ReadError[] readErrors = IntStream.range(0, 1 + random.nextInt(10)) + .mapToObj(i -> new LineReader.ReadError(HIGHLIGHTING, line + i)) + .toArray(LineReader.ReadError[]::new); + when(system2.now()).thenReturn(timeStamp); + + Arrays.stream(readErrors).forEach(readError -> underTest.addWarning(file, readError)); + + verifyZeroInteractions(taskMessages); + + underTest.commitWarnings(); + + verify(taskMessages, times(1)) + .add(new CeTaskMessages.Message("Inconsistent highlighting data detected on file '" + path + "'. " + + "File source may have been modified while analysis was running.", timeStamp)); + } + + @Test + public void create_highlighting_warning_when_any_number_of_read_error_for_less_than_5_files() { + int fileCount = 2 + random.nextInt(3); + Component[] files = IntStream.range(0, fileCount) + .mapToObj(i -> ReportComponent.builder(Component.Type.FILE, i) + .setUuid("uuid_" + i) + .setPath(path + "_" + i) + .build()) + .toArray(Component[]::new); + when(system2.now()).thenReturn(timeStamp); + + Arrays.stream(files).forEach(file -> IntStream.range(0, 1 + random.nextInt(10)) + .forEach(i -> underTest.addWarning(file, new LineReader.ReadError(HIGHLIGHTING, line + i)))); + + verifyZeroInteractions(taskMessages); + + underTest.commitWarnings(); + + String expectedMessage = "Inconsistent highlighting data detected on some files (" + fileCount + " in total). " + + "File source may have been modified while analysis was running." + + Arrays.stream(files).map(f -> f.getReportAttributes().getPath()).collect(Collectors.joining("\n ° ", "\n ° ", "")); + verify(taskMessages, times(1)) + .add(new CeTaskMessages.Message(expectedMessage, timeStamp)); + } + + @Test + public void create_highlighting_warning_when_any_number_of_read_error_for_more_than_5_files_only_the_5_first_by_ref() { + int fileCount = 6 + random.nextInt(4); + Component[] files = IntStream.range(0, fileCount) + .mapToObj(i -> ReportComponent.builder(Component.Type.FILE, i) + .setUuid("uuid_" + i) + .setPath(path + "_" + i) + .build()) + .toArray(Component[]::new); + when(system2.now()).thenReturn(timeStamp); + + Arrays.stream(files).forEach(file -> IntStream.range(0, 1 + random.nextInt(10)) + .forEach(i -> underTest.addWarning(file, new LineReader.ReadError(HIGHLIGHTING, line + i)))); + + verifyZeroInteractions(taskMessages); + + underTest.commitWarnings(); + + String expectedMessage = "Inconsistent highlighting data detected on some files (" + fileCount + " in total). " + + "File source may have been modified while analysis was running." + + Arrays.stream(files).limit(5).map(f -> f.getReportAttributes().getPath()).collect(Collectors.joining("\n ° ", "\n ° ", "")); + verify(taskMessages, times(1)) + .add(new CeTaskMessages.Message(expectedMessage, timeStamp)); + } + + @Test + @UseDataProvider("anyDataButHighlight") + public void creates_no_warning_when_read_error_for_anything_but_highlighting(LineReader.Data data) { + ReportComponent file = ReportComponent.builder(Component.Type.FILE, 1) + .setUuid("uuid") + .setPath(path) + .build(); + LineReader.ReadError readError = new LineReader.ReadError(data, line); + when(system2.now()).thenReturn(timeStamp); + + underTest.addWarning(file, readError); + + verifyZeroInteractions(taskMessages); + + underTest.commitWarnings(); + + verifyZeroInteractions(taskMessages); + } + + @DataProvider + public static Object[][] anyDataButHighlight() { + return Arrays.stream(LineReader.Data.values()) + .filter(t -> t != HIGHLIGHTING) + .map(t -> new Object[] {t}) + .toArray(Object[][]::new); + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/LineReadersImplTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/LineReadersImplTest.java new file mode 100644 index 00000000000..e187b0529f1 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/LineReadersImplTest.java @@ -0,0 +1,146 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.ce.task.projectanalysis.source; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.Random; +import java.util.function.Consumer; +import java.util.stream.IntStream; +import org.junit.Test; +import org.sonar.ce.task.projectanalysis.scm.Changeset; +import org.sonar.ce.task.projectanalysis.source.linereader.LineReader; +import org.sonar.ce.task.projectanalysis.source.linereader.LineReader.ReadError; +import org.sonar.ce.task.projectanalysis.source.linereader.ScmLineReader; +import org.sonar.core.util.CloseableIterator; +import org.sonar.db.protobuf.DbFileSources; + +import static java.util.Arrays.asList; +import static java.util.Arrays.stream; +import static java.util.stream.Collectors.toList; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; + +public class LineReadersImplTest { + private static final Consumer NOOP_READ_ERROR_CONSUMER = readError -> { + }; + + @Test + public void close_closes_all_closeables() { + LineReader r1 = mock(LineReader.class); + LineReader r2 = mock(LineReader.class); + CloseableIterator c1 = newCloseableIteratorMock(); + CloseableIterator c2 = newCloseableIteratorMock(); + + SourceLineReadersFactory.LineReaders lineReaders = new SourceLineReadersFactory.LineReadersImpl(asList(r1, r2), null, asList(c1, c2)); + lineReaders.close(); + + verify(c1).close(); + verify(c2).close(); + verifyNoMoreInteractions(c1, c2); + verifyZeroInteractions(r1, r2); + } + + @Test + public void read_calls_all_readers() { + LineReader r1 = mock(LineReader.class); + LineReader r2 = mock(LineReader.class); + CloseableIterator c1 = newCloseableIteratorMock(); + CloseableIterator c2 = newCloseableIteratorMock(); + + SourceLineReadersFactory.LineReadersImpl lineReaders = new SourceLineReadersFactory.LineReadersImpl(asList(r1, r2), null, asList(c1, c2)); + DbFileSources.Line.Builder builder = DbFileSources.Line.newBuilder(); + lineReaders.read(builder, NOOP_READ_ERROR_CONSUMER); + + verify(r1).read(builder); + verify(r2).read(builder); + verifyNoMoreInteractions(r1, r2); + verifyZeroInteractions(c1, c2); + } + + @Test + public void read_calls_ReaderError_consumer_with_each_read_error_returned_by_all_readers() { + int readErrorCount = 2 + 2 * new Random().nextInt(10); + int halfCount = readErrorCount / 2; + ReadError[] expectedReadErrors = IntStream.range(0, readErrorCount) + .mapToObj(i -> new ReadError(LineReader.Data.SYMBOLS, i)) + .toArray(ReadError[]::new); + LineReader[] lineReaders = IntStream.range(0, halfCount) + .mapToObj(i -> { + LineReader lineReader = mock(LineReader.class); + when(lineReader.read(any())) + .thenReturn(Optional.of(expectedReadErrors[i])) + .thenReturn(Optional.of(expectedReadErrors[i + halfCount])) + .thenReturn(Optional.empty()); + return lineReader; + }) + .toArray(LineReader[]::new); + DbFileSources.Line.Builder builder = DbFileSources.Line.newBuilder(); + SourceLineReadersFactory.LineReadersImpl underTest = new SourceLineReadersFactory.LineReadersImpl( + stream(lineReaders).collect(toList()), null, Collections.emptyList()); + List readErrors = new ArrayList<>(); + + // calls first mocked result of each Reader mock => we get first half read errors + underTest.read(builder, readErrors::add); + assertThat(readErrors).contains(stream(expectedReadErrors).limit(halfCount).toArray(ReadError[]::new)); + + // calls first mocked result of each Reader mock => we get second half read errors + readErrors.clear(); + underTest.read(builder, readErrors::add); + assertThat(readErrors).contains(stream(expectedReadErrors).skip(halfCount).toArray(ReadError[]::new)); + + // calls third mocked result of each Reader mock (empty) => consumer is never called => empty list + readErrors.clear(); + underTest.read(builder, readErrors::add); + assertThat(readErrors).isEmpty(); + } + + @Test + public void getLatestChangeWithRevision_delegates_to_ScmLineReader_if_non_null() { + ScmLineReader scmLineReader = mock(ScmLineReader.class); + Changeset changeset = Changeset.newChangesetBuilder().setDate(0L).build(); + when(scmLineReader.getLatestChangeWithRevision()).thenReturn(changeset); + + SourceLineReadersFactory.LineReaders lineReaders = new SourceLineReadersFactory.LineReadersImpl(Collections.emptyList(), scmLineReader, Collections.emptyList()); + assertThat(lineReaders.getLatestChangeWithRevision()).isEqualTo(changeset); + + verify(scmLineReader).getLatestChangeWithRevision(); + verifyNoMoreInteractions(scmLineReader); + } + + @Test + public void getLatestChangeWithRevision_returns_null_if_ScmLineReader_is_null() { + SourceLineReadersFactory.LineReaders lineReaders = new SourceLineReadersFactory.LineReadersImpl(Collections.emptyList(), null, Collections.emptyList()); + assertThat(lineReaders.getLatestChangeWithRevision()).isNull(); + } + + @SuppressWarnings("unchecked") + private static CloseableIterator newCloseableIteratorMock() { + return (CloseableIterator)mock(CloseableIterator.class); + } + +} diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/SourceLineReadersFactoryTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/SourceLineReadersFactoryTest.java index 052baadae3d..0091a6fb78e 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/SourceLineReadersFactoryTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/SourceLineReadersFactoryTest.java @@ -19,9 +19,6 @@ */ package org.sonar.ce.task.projectanalysis.source; -import java.util.Arrays; -import java.util.Collections; -import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.sonar.ce.task.projectanalysis.batch.BatchReportReaderRule; @@ -30,27 +27,18 @@ import org.sonar.ce.task.projectanalysis.component.FileAttributes; import org.sonar.ce.task.projectanalysis.component.ReportComponent; import org.sonar.ce.task.projectanalysis.component.TreeRootHolderRule; import org.sonar.ce.task.projectanalysis.duplication.DuplicationRepositoryRule; -import org.sonar.ce.task.projectanalysis.scm.Changeset; import org.sonar.ce.task.projectanalysis.scm.ScmInfoRepositoryRule; -import org.sonar.ce.task.projectanalysis.source.linereader.LineReader; -import org.sonar.ce.task.projectanalysis.source.linereader.ScmLineReader; -import org.sonar.core.util.CloseableIterator; -import org.sonar.db.protobuf.DbFileSources; +import org.sonar.ce.task.projectanalysis.source.SourceLineReadersFactory.LineReadersImpl; import org.sonar.scanner.protocol.output.ScannerReport; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.verifyZeroInteractions; -import static org.mockito.Mockito.when; public class SourceLineReadersFactoryTest { private static final int FILE1_REF = 3; private static final String PROJECT_UUID = "PROJECT"; private static final String PROJECT_KEY = "PROJECT_KEY"; private static final String FILE1_UUID = "FILE1"; - private static final long NOW = 123456789L; @Rule public TreeRootHolderRule treeRootHolder = new TreeRootHolderRule(); @@ -60,73 +48,21 @@ public class SourceLineReadersFactoryTest { public ScmInfoRepositoryRule scmInfoRepository = new ScmInfoRepositoryRule(); @Rule public DuplicationRepositoryRule duplicationRepository = DuplicationRepositoryRule.create(treeRootHolder); - private NewLinesRepository newLinesRepository = mock(NewLinesRepository.class); - private SourceLineReadersFactory underTest; + private NewLinesRepository newLinesRepository = mock(NewLinesRepository.class); - @Before - public void setUp() { - underTest = new SourceLineReadersFactory(reportReader, scmInfoRepository, duplicationRepository, newLinesRepository); - } + private SourceLineReadersFactory underTest = new SourceLineReadersFactory(reportReader, scmInfoRepository, duplicationRepository, newLinesRepository); @Test public void should_create_readers() { initBasicReport(10); - SourceLineReadersFactory.LineReaders lineReaders = underTest.getLineReaders(fileComponent()); + LineReadersImpl lineReaders = (LineReadersImpl) underTest.getLineReaders(fileComponent()); assertThat(lineReaders).isNotNull(); assertThat(lineReaders.closeables).hasSize(3); assertThat(lineReaders.readers).hasSize(5); } - @Test - public void line_readers_should_close_all_closeables() { - LineReader r1 = mock(LineReader.class); - LineReader r2 = mock(LineReader.class); - CloseableIterator c1 = mock(CloseableIterator.class); - CloseableIterator c2 = mock(CloseableIterator.class); - - SourceLineReadersFactory.LineReaders lineReaders = new SourceLineReadersFactory.LineReaders(Arrays.asList(r1, r2), null, Arrays.asList(c1, c2)); - lineReaders.close(); - - verify(c1).close(); - verify(c2).close(); - verifyNoMoreInteractions(c1, c2); - verifyZeroInteractions(r1, r2); - } - - @Test - public void line_readers_should_call_all_readers() { - LineReader r1 = mock(LineReader.class); - LineReader r2 = mock(LineReader.class); - CloseableIterator c1 = mock(CloseableIterator.class); - CloseableIterator c2 = mock(CloseableIterator.class); - - SourceLineReadersFactory.LineReaders lineReaders = new SourceLineReadersFactory.LineReaders(Arrays.asList(r1, r2), null, Arrays.asList(c1, c2)); - DbFileSources.Line.Builder builder = DbFileSources.Line.newBuilder(); - lineReaders.read(builder); - - verify(r1).read(builder); - verify(r2).read(builder); - verifyNoMoreInteractions(r1, r2); - verifyZeroInteractions(c1, c2); - } - - @Test - public void should_delegate_latest_changeset() { - ScmLineReader scmLineReader = mock(ScmLineReader.class); - Changeset changeset = Changeset.newChangesetBuilder().setDate(0L).build(); - when(scmLineReader.getLatestChangeWithRevision()).thenReturn(changeset); - SourceLineReadersFactory.LineReaders lineReaders = new SourceLineReadersFactory.LineReaders(Collections.emptyList(), scmLineReader, Collections.emptyList()); - assertThat(lineReaders.getLatestChangeWithRevision()).isEqualTo(changeset); - } - - @Test - public void should_not_delegate_latest_changeset() { - SourceLineReadersFactory.LineReaders lineReaders = new SourceLineReadersFactory.LineReaders(Collections.emptyList(), null, Collections.emptyList()); - assertThat(lineReaders.getLatestChangeWithRevision()).isNull(); - } - private Component fileComponent() { return ReportComponent.builder(Component.Type.FILE, FILE1_REF).build(); } @@ -154,10 +90,6 @@ public class SourceLineReadersFactoryTest { .setType(ScannerReport.Component.ComponentType.FILE) .setLines(numberOfLines) .build()); - - // for (int i = 1; i <= numberOfLines; i++) { - // fileSourceRepository.addLine(FILE1_REF, "line" + i); - // } } } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/linereader/CoverageLineReaderTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/linereader/CoverageLineReaderTest.java index a126a0b9b1d..484ede7a27c 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/linereader/CoverageLineReaderTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/linereader/CoverageLineReaderTest.java @@ -39,7 +39,7 @@ public class CoverageLineReaderTest { .build()).iterator()); DbFileSources.Line.Builder lineBuilder = DbFileSources.Data.newBuilder().addLinesBuilder().setLine(1); - computeCoverageLine.read(lineBuilder); + assertThat(computeCoverageLine.read(lineBuilder)).isEmpty(); assertThat(lineBuilder.getLineHits()).isEqualTo(1); assertThat(lineBuilder.getConditions()).isEqualTo(10); @@ -56,7 +56,7 @@ public class CoverageLineReaderTest { .build()).iterator()); DbFileSources.Line.Builder lineBuilder = DbFileSources.Data.newBuilder().addLinesBuilder().setLine(1); - computeCoverageLine.read(lineBuilder); + assertThat(computeCoverageLine.read(lineBuilder)).isEmpty(); assertThat(lineBuilder.hasLineHits()).isFalse(); assertThat(lineBuilder.getConditions()).isEqualTo(10); @@ -70,7 +70,7 @@ public class CoverageLineReaderTest { .build()).iterator()); DbFileSources.Line.Builder lineBuilder = DbFileSources.Data.newBuilder().addLinesBuilder().setLine(1); - computeCoverageLine.read(lineBuilder); + assertThat(computeCoverageLine.read(lineBuilder)).isEmpty(); assertThat(lineBuilder.hasLineHits()).isTrue(); assertThat(lineBuilder.getLineHits()).isEqualTo(0); @@ -81,7 +81,7 @@ public class CoverageLineReaderTest { CoverageLineReader computeCoverageLine = new CoverageLineReader(Collections.emptyList().iterator()); DbFileSources.Line.Builder lineBuilder = DbFileSources.Data.newBuilder().addLinesBuilder().setLine(1); - computeCoverageLine.read(lineBuilder); + assertThat(computeCoverageLine.read(lineBuilder)).isEmpty(); assertThat(lineBuilder.hasLineHits()).isFalse(); assertThat(lineBuilder.hasConditions()).isFalse(); @@ -141,7 +141,7 @@ public class CoverageLineReaderTest { .build()).iterator()); DbFileSources.Line.Builder lineBuilder = DbFileSources.Data.newBuilder().addLinesBuilder().setLine(1); - computeCoverageLine.read(lineBuilder); + assertThat(computeCoverageLine.read(lineBuilder)).isEmpty(); assertThat(lineBuilder.hasDeprecatedUtLineHits()).isFalse(); assertThat(lineBuilder.hasDeprecatedUtConditions()).isFalse(); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/linereader/DuplicationLineReaderTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/linereader/DuplicationLineReaderTest.java index 5d538a7ac32..9d86533fa23 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/linereader/DuplicationLineReaderTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/linereader/DuplicationLineReaderTest.java @@ -47,7 +47,7 @@ public class DuplicationLineReaderTest { public void read_nothing() { DuplicationLineReader reader = new DuplicationLineReader(Collections.emptySet()); - reader.read(line1); + assertThat(reader.read(line1)).isEmpty(); assertThat(line1.getDuplicationList()).isEmpty(); } @@ -56,10 +56,10 @@ public class DuplicationLineReaderTest { public void read_duplication_with_duplicates_on_same_file() { DuplicationLineReader reader = duplicationLineReader(duplication(1, 2, innerDuplicate(3, 4))); - reader.read(line1); - reader.read(line2); - reader.read(line3); - reader.read(line4); + assertThat(reader.read(line1)).isEmpty(); + assertThat(reader.read(line2)).isEmpty(); + assertThat(reader.read(line3)).isEmpty(); + assertThat(reader.read(line4)).isEmpty(); assertThat(line1.getDuplicationList()).containsExactly(1); assertThat(line2.getDuplicationList()).containsExactly(1); @@ -74,10 +74,10 @@ public class DuplicationLineReaderTest { 1, 2, new InProjectDuplicate(fileComponent(1).build(), new TextBlock(3, 4)))); - reader.read(line1); - reader.read(line2); - reader.read(line3); - reader.read(line4); + assertThat(reader.read(line1)).isEmpty(); + assertThat(reader.read(line2)).isEmpty(); + assertThat(reader.read(line3)).isEmpty(); + assertThat(reader.read(line4)).isEmpty(); assertThat(line1.getDuplicationList()).containsExactly(1); assertThat(line2.getDuplicationList()).containsExactly(1); @@ -92,10 +92,10 @@ public class DuplicationLineReaderTest { 1, 2, new CrossProjectDuplicate("other-component-key-from-another-project", new TextBlock(3, 4)))); - reader.read(line1); - reader.read(line2); - reader.read(line3); - reader.read(line4); + assertThat(reader.read(line1)).isEmpty(); + assertThat(reader.read(line2)).isEmpty(); + assertThat(reader.read(line3)).isEmpty(); + assertThat(reader.read(line4)).isEmpty(); assertThat(line1.getDuplicationList()).containsExactly(1); assertThat(line2.getDuplicationList()).containsExactly(1); @@ -113,10 +113,10 @@ public class DuplicationLineReaderTest { 1, 2, innerDuplicate(3, 4))); - reader.read(line1); - reader.read(line2); - reader.read(line3); - reader.read(line4); + assertThat(reader.read(line1)).isEmpty(); + assertThat(reader.read(line2)).isEmpty(); + assertThat(reader.read(line3)).isEmpty(); + assertThat(reader.read(line4)).isEmpty(); assertThat(line1.getDuplicationList()).containsExactly(1, 2); assertThat(line2.getDuplicationList()).containsExactly(2, 3); @@ -134,10 +134,10 @@ public class DuplicationLineReaderTest { 1, 1, innerDuplicate(3, 3))); - reader.read(line1); - reader.read(line2); - reader.read(line3); - reader.read(line4); + assertThat(reader.read(line1)).isEmpty(); + assertThat(reader.read(line2)).isEmpty(); + assertThat(reader.read(line3)).isEmpty(); + assertThat(reader.read(line4)).isEmpty(); assertThat(line1.getDuplicationList()).containsExactly(1); assertThat(line2.getDuplicationList()).containsExactly(2); @@ -155,10 +155,10 @@ public class DuplicationLineReaderTest { 1, 1, innerDuplicate(4, 4))); - reader.read(line1); - reader.read(line2); - reader.read(line3); - reader.read(line4); + assertThat(reader.read(line1)).isEmpty(); + assertThat(reader.read(line2)).isEmpty(); + assertThat(reader.read(line3)).isEmpty(); + assertThat(reader.read(line4)).isEmpty(); assertThat(line1.getDuplicationList()).containsExactly(1, 2); assertThat(line2.getDuplicationList()).containsExactly(2); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/linereader/HighlightingLineReaderTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/linereader/HighlightingLineReaderTest.java index 85d4881d457..f6882b07f57 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/linereader/HighlightingLineReaderTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/linereader/HighlightingLineReaderTest.java @@ -41,6 +41,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.sonar.api.utils.log.LoggerLevel.WARN; import static org.sonar.ce.task.projectanalysis.component.ReportComponent.builder; +import static org.sonar.ce.task.projectanalysis.source.linereader.LineReader.Data.HIGHLIGHTING; import static org.sonar.db.protobuf.DbFileSources.Data.newBuilder; import static org.sonar.scanner.protocol.output.ScannerReport.SyntaxHighlightingRule.HighlightingType.ANNOTATION; import static org.sonar.scanner.protocol.output.ScannerReport.SyntaxHighlightingRule.HighlightingType.COMMENT; @@ -82,7 +83,7 @@ public class HighlightingLineReaderTest { HighlightingLineReader highlightingLineReader = newReader(Collections.emptyMap()); DbFileSources.Line.Builder lineBuilder = newBuilder().addLinesBuilder().setLine(1); - highlightingLineReader.read(lineBuilder); + assertThat(highlightingLineReader.read(lineBuilder)).isEmpty(); assertThat(lineBuilder.hasHighlighting()).isFalse(); } @@ -92,7 +93,7 @@ public class HighlightingLineReaderTest { HighlightingLineReader highlightingLineReader = newReader(of( newSingleLineTextRangeWithExpectingLabel(LINE_1, RANGE_LABEL_1), ANNOTATION)); - highlightingLineReader.read(line1); + assertThat(highlightingLineReader.read(line1)).isEmpty(); assertThat(line1.getHighlighting()).isEqualTo(RANGE_LABEL_1 + ",a"); } @@ -104,10 +105,10 @@ public class HighlightingLineReaderTest { newSingleLineTextRangeWithExpectingLabel(LINE_2, RANGE_LABEL_2), COMMENT, newSingleLineTextRangeWithExpectingLabel(LINE_4, RANGE_LABEL_3), CONSTANT)); - highlightingLineReader.read(line1); - highlightingLineReader.read(line2); - highlightingLineReader.read(line3); - highlightingLineReader.read(line4); + assertThat(highlightingLineReader.read(line1)).isEmpty(); + assertThat(highlightingLineReader.read(line2)).isEmpty(); + assertThat(highlightingLineReader.read(line3)).isEmpty(); + assertThat(highlightingLineReader.read(line4)).isEmpty(); assertThat(line1.getHighlighting()).isEqualTo(RANGE_LABEL_1 + ",a"); assertThat(line2.getHighlighting()).isEqualTo(RANGE_LABEL_2 + ",cd"); @@ -145,7 +146,7 @@ public class HighlightingLineReaderTest { private DbFileSources.Line.Builder addSourceLine(HighlightingLineReader highlightingLineReader, int line, String source) { DbFileSources.Line.Builder lineBuilder = sourceData.addLinesBuilder().setSource(source).setLine(line); - highlightingLineReader.read(lineBuilder); + assertThat(highlightingLineReader.read(lineBuilder)).isEmpty(); return lineBuilder; } @@ -170,7 +171,7 @@ public class HighlightingLineReaderTest { newSingleLineTextRangeWithExpectingLabel(LINE_1, RANGE_LABEL_1), ANNOTATION, newSingleLineTextRangeWithExpectingLabel(LINE_1, RANGE_LABEL_2), COMMENT)); - highlightingLineReader.read(line1); + assertThat(highlightingLineReader.read(line1)).isEmpty(); assertThat(line1.getHighlighting()).isEqualTo(RANGE_LABEL_1 + ",a;" + RANGE_LABEL_2 + ",cd"); } @@ -185,10 +186,10 @@ public class HighlightingLineReaderTest { HighlightingLineReader highlightingLineReader = newReader(of(textRange, ANNOTATION)); - highlightingLineReader.read(line1); + assertThat(highlightingLineReader.read(line1)).isEmpty(); DbFileSources.Line.Builder line2 = sourceData.addLinesBuilder().setSource("line 2").setLine(2); - highlightingLineReader.read(line2); - highlightingLineReader.read(line3); + assertThat(highlightingLineReader.read(line2)).isEmpty(); + assertThat(highlightingLineReader.read(line3)).isEmpty(); assertThat(line1.getHighlighting()).isEqualTo(RANGE_LABEL_1 + ",a"); assertThat(line2.getHighlighting()).isEqualTo(RANGE_LABEL_2 + ",a"); @@ -215,10 +216,10 @@ public class HighlightingLineReaderTest { textRange2, HIGHLIGHTING_STRING, textRange3, COMMENT)); - highlightingLineReader.read(line1); - highlightingLineReader.read(line2); - highlightingLineReader.read(line3); - highlightingLineReader.read(line4); + assertThat(highlightingLineReader.read(line1)).isEmpty(); + assertThat(highlightingLineReader.read(line2)).isEmpty(); + assertThat(highlightingLineReader.read(line3)).isEmpty(); + assertThat(highlightingLineReader.read(line4)).isEmpty(); assertThat(line1.getHighlighting()).isEqualTo(RANGE_LABEL_1 + ",a"); assertThat(line2.getHighlighting()).isEqualTo(RANGE_LABEL_2 + ",a;" + RANGE_LABEL_2 + ",s;" + RANGE_LABEL_5 + ",cd"); @@ -234,9 +235,9 @@ public class HighlightingLineReaderTest { HighlightingLineReader highlightingLineReader = newReader(of(textRange, ANNOTATION)); - highlightingLineReader.read(line1); - highlightingLineReader.read(line2); - highlightingLineReader.read(line3); + assertThat(highlightingLineReader.read(line1)).isEmpty(); + assertThat(highlightingLineReader.read(line2)).isEmpty(); + assertThat(highlightingLineReader.read(line3)).isEmpty(); assertThat(line1.getHighlighting()).isEqualTo(RANGE_LABEL_1 + ",a"); // Nothing should be set on line 2 @@ -253,8 +254,9 @@ public class HighlightingLineReaderTest { textRange1, HighlightingType.ANNOTATION, newSingleLineTextRangeWithExpectingLabel(LINE_2, RANGE_LABEL_1), HIGHLIGHTING_STRING)); - highlightingLineReader.read(line1); - highlightingLineReader.read(line2); + LineReader.ReadError readErrorLine1 = new LineReader.ReadError(HIGHLIGHTING, LINE_1); + assertThat(highlightingLineReader.read(line1)).contains(readErrorLine1); + assertThat(highlightingLineReader.read(line2)).contains(readErrorLine1); assertNoHighlighting(); assertThat(logTester.logs(WARN)).isNotEmpty(); @@ -264,16 +266,21 @@ public class HighlightingLineReaderTest { public void keep_existing_processed_highlighting_when_range_offset_converter_throw_RangeOffsetConverterException() { TextRange textRange2 = newTextRange(LINE_2, LINE_2); doThrow(RangeOffsetConverterException.class).when(rangeOffsetConverter).offsetToString(textRange2, LINE_2, DEFAULT_LINE_LENGTH); + TextRange textRange3 = newTextRange(LINE_3, LINE_3); HighlightingLineReader highlightingLineReader = newReader(of( newSingleLineTextRangeWithExpectingLabel(LINE_1, RANGE_LABEL_1), ANNOTATION, - textRange2, HIGHLIGHTING_STRING)); + textRange2, HIGHLIGHTING_STRING, + textRange3, COMMENT)); - highlightingLineReader.read(line1); - highlightingLineReader.read(line2); + assertThat(highlightingLineReader.read(line1)).isEmpty(); + LineReader.ReadError readErrorLine2 = new LineReader.ReadError(HIGHLIGHTING, LINE_2); + assertThat(highlightingLineReader.read(line2)).contains(readErrorLine2); + assertThat(highlightingLineReader.read(line3)).contains(readErrorLine2); assertThat(line1.hasHighlighting()).isTrue(); assertThat(line2.hasHighlighting()).isFalse(); + assertThat(line3.hasHighlighting()).isFalse(); assertThat(logTester.logs(WARN)).isNotEmpty(); } @@ -283,7 +290,8 @@ public class HighlightingLineReaderTest { doThrow(RangeOffsetConverterException.class).when(rangeOffsetConverter).offsetToString(textRange1, LINE_1, DEFAULT_LINE_LENGTH); HighlightingLineReader highlightingLineReader = newReader(of(textRange1, ANNOTATION)); - highlightingLineReader.read(line1); + assertThat(highlightingLineReader.read(line1)) + .contains(new LineReader.ReadError(HIGHLIGHTING, 1)); assertThat(logTester.logs(WARN)).containsOnly("Inconsistency detected in Highlighting data. Highlighting will be ignored for file 'FILE_KEY'"); } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/linereader/ScmLineReaderTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/linereader/ScmLineReaderTest.java index 548f061abbc..e123051684f 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/linereader/ScmLineReaderTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/linereader/ScmLineReaderTest.java @@ -44,7 +44,7 @@ public class ScmLineReaderTest { ScmLineReader lineScm = new ScmLineReader(scmInfo); DbFileSources.Line.Builder lineBuilder = DbFileSources.Data.newBuilder().addLinesBuilder().setLine(1); - lineScm.read(lineBuilder); + assertThat(lineScm.read(lineBuilder)).isEmpty(); assertThat(lineBuilder.getScmAuthor()).isEqualTo("john"); assertThat(lineBuilder.getScmDate()).isEqualTo(123_456_789L); @@ -61,7 +61,7 @@ public class ScmLineReaderTest { ScmLineReader lineScm = new ScmLineReader(scmInfo); DbFileSources.Line.Builder lineBuilder = DbFileSources.Data.newBuilder().addLinesBuilder().setLine(1); - lineScm.read(lineBuilder); + assertThat(lineScm.read(lineBuilder)).isEmpty(); assertThat(lineBuilder.hasScmAuthor()).isFalse(); assertThat(lineBuilder.getScmDate()).isEqualTo(123456789L); @@ -116,7 +116,7 @@ public class ScmLineReaderTest { private void readLineAndAssertLatestChanges(ScmLineReader lineScm, int line, Changeset expectedChangeset, Changeset expectedChangesetWithRevision) { DbFileSources.Line.Builder lineBuilder = DbFileSources.Data.newBuilder().addLinesBuilder().setLine(line); - lineScm.read(lineBuilder); + assertThat(lineScm.read(lineBuilder)).isEmpty(); assertThat(lineScm.getLatestChange()).isSameAs(expectedChangeset); assertThat(lineScm.getLatestChangeWithRevision()).isSameAs(expectedChangesetWithRevision); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/linereader/SymbolsLineReaderTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/linereader/SymbolsLineReaderTest.java index d715dd001b5..a93348d9795 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/linereader/SymbolsLineReaderTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/source/linereader/SymbolsLineReaderTest.java @@ -34,6 +34,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.sonar.api.utils.log.LoggerLevel.WARN; import static org.sonar.ce.task.projectanalysis.component.ReportComponent.builder; +import static org.sonar.ce.task.projectanalysis.source.linereader.LineReader.Data.SYMBOLS; public class SymbolsLineReaderTest { @@ -72,7 +73,7 @@ public class SymbolsLineReaderTest { public void read_nothing() { SymbolsLineReader symbolsLineReader = newReader(); - symbolsLineReader.read(line1); + assertThat(symbolsLineReader.read(line1)).isEmpty(); assertThat(line1.getSymbols()).isEmpty(); } @@ -83,9 +84,9 @@ public class SymbolsLineReaderTest { newSingleLineTextRangeWithExpectedLabel(LINE_1, OFFSET_2, OFFSET_4, RANGE_LABEL_1), newSingleLineTextRangeWithExpectedLabel(LINE_3, OFFSET_1, OFFSET_3, RANGE_LABEL_2))); - symbolsLineReader.read(line1); - symbolsLineReader.read(line2); - symbolsLineReader.read(line3); + assertThat(symbolsLineReader.read(line1)).isEmpty(); + assertThat(symbolsLineReader.read(line2)).isEmpty(); + assertThat(symbolsLineReader.read(line3)).isEmpty(); assertThat(line1.getSymbols()).isEqualTo(RANGE_LABEL_1 + ",1"); assertThat(line2.getSymbols()).isEmpty(); @@ -98,7 +99,7 @@ public class SymbolsLineReaderTest { newSingleLineTextRangeWithExpectedLabel(LINE_1, OFFSET_0, OFFSET_1, RANGE_LABEL_1), newSingleLineTextRangeWithExpectedLabel(LINE_1, OFFSET_2, OFFSET_3, RANGE_LABEL_2))); - symbolsLineReader.read(line1); + assertThat(symbolsLineReader.read(line1)).isEmpty(); assertThat(line1.getSymbols()).isEqualTo(RANGE_LABEL_1 + ",1;" + RANGE_LABEL_2 + ",1"); } @@ -110,9 +111,9 @@ public class SymbolsLineReaderTest { newSingleLineTextRangeWithExpectedLabel(LINE_3, OFFSET_1, OFFSET_3, RANGE_LABEL_2), newSingleLineTextRangeWithExpectedLabel(LINE_2, OFFSET_0, OFFSET_2, RANGE_LABEL_3))); - symbolsLineReader.read(line1); - symbolsLineReader.read(line2); - symbolsLineReader.read(line3); + assertThat(symbolsLineReader.read(line1)).isEmpty(); + assertThat(symbolsLineReader.read(line2)).isEmpty(); + assertThat(symbolsLineReader.read(line3)).isEmpty(); assertThat(line1.getSymbols()).isEqualTo(RANGE_LABEL_1 + ",1"); assertThat(line2.getSymbols()).isEqualTo(RANGE_LABEL_3 + ",1"); @@ -126,8 +127,8 @@ public class SymbolsLineReaderTest { newSingleLineTextRangeWithExpectedLabel(LINE_2, OFFSET_0, OFFSET_1, RANGE_LABEL_2), newSingleLineTextRangeWithExpectedLabel(LINE_2, OFFSET_2, OFFSET_3, RANGE_LABEL_3))); - symbolsLineReader.read(line1); - symbolsLineReader.read(line2); + assertThat(symbolsLineReader.read(line1)).isEmpty(); + assertThat(symbolsLineReader.read(line2)).isEmpty(); assertThat(line1.getSymbols()).isEqualTo(RANGE_LABEL_1 + ",1"); assertThat(line2.getSymbols()).isEqualTo(RANGE_LABEL_2 + ",1;" + RANGE_LABEL_3 + ",1"); @@ -139,8 +140,8 @@ public class SymbolsLineReaderTest { newSingleLineTextRangeWithExpectedLabel(LINE_2, OFFSET_3, OFFSET_4, RANGE_LABEL_1), newSingleLineTextRangeWithExpectedLabel(LINE_1, OFFSET_1, OFFSET_2, RANGE_LABEL_2))); - symbolsLineReader.read(line1); - symbolsLineReader.read(line2); + assertThat(symbolsLineReader.read(line1)).isEmpty(); + assertThat(symbolsLineReader.read(line2)).isEmpty(); assertThat(line1.getSymbols()).isEqualTo(RANGE_LABEL_2 + ",1"); assertThat(line2.getSymbols()).isEqualTo(RANGE_LABEL_1 + ",1"); @@ -156,9 +157,9 @@ public class SymbolsLineReaderTest { newSingleLineTextRangeWithExpectedLabel(LINE_1, OFFSET_3, OFFSET_4, RANGE_LABEL_3), newSingleLineTextRangeWithExpectedLabel(LINE_3, OFFSET_0, OFFSET_1, RANGE_LABEL_4))); - symbolsLineReader.read(line1); - symbolsLineReader.read(line2); - symbolsLineReader.read(line3); + assertThat(symbolsLineReader.read(line1)).isEmpty(); + assertThat(symbolsLineReader.read(line2)).isEmpty(); + assertThat(symbolsLineReader.read(line3)).isEmpty(); assertThat(line1.getSymbols()).isEqualTo(RANGE_LABEL_1 + ",1;" + RANGE_LABEL_3 + ",2"); assertThat(line2.getSymbols()).isEmpty(); @@ -176,9 +177,9 @@ public class SymbolsLineReaderTest { newSingleLineTextRangeWithExpectedLabel(LINE_1, OFFSET_0, OFFSET_1, RANGE_LABEL_2), newSingleLineTextRangeWithExpectedLabel(LINE_3, OFFSET_0, OFFSET_1, RANGE_LABEL_2))); - symbolsLineReader.read(line1); - symbolsLineReader.read(line2); - symbolsLineReader.read(line3); + assertThat(symbolsLineReader.read(line1)).isEmpty(); + assertThat(symbolsLineReader.read(line2)).isEmpty(); + assertThat(symbolsLineReader.read(line3)).isEmpty(); assertThat(line1.getSymbols()).isEqualTo(RANGE_LABEL_2 + ",1;" + RANGE_LABEL_1 + ",2"); assertThat(line2.getSymbols()).isEmpty(); @@ -195,7 +196,7 @@ public class SymbolsLineReaderTest { newSingleLineTextRangeWithExpectedLabel(LINE_1, OFFSET_0, OFFSET_1, RANGE_LABEL_1), newSingleLineTextRangeWithExpectedLabel(LINE_3, OFFSET_0, OFFSET_1, RANGE_LABEL_1))); - symbolsLineReader.read(line1); + assertThat(symbolsLineReader.read(line1)).isEmpty(); symbolsLineReader.read(line2); symbolsLineReader.read(line3); @@ -216,10 +217,10 @@ public class SymbolsLineReaderTest { SymbolsLineReader symbolsLineReader = newReader(newSymbol(declaration, reference)); - symbolsLineReader.read(line1); - symbolsLineReader.read(line2); - symbolsLineReader.read(line3); - symbolsLineReader.read(line4); + assertThat(symbolsLineReader.read(line1)).isEmpty(); + assertThat(symbolsLineReader.read(line2)).isEmpty(); + assertThat(symbolsLineReader.read(line3)).isEmpty(); + assertThat(symbolsLineReader.read(line4)).isEmpty(); assertThat(line1.getSymbols()).isEqualTo(RANGE_LABEL_1 + ",1"); assertThat(line2.getSymbols()).isEqualTo(RANGE_LABEL_2 + ",1"); @@ -236,10 +237,10 @@ public class SymbolsLineReaderTest { SymbolsLineReader symbolsLineReader = newReader(newSymbol(declaration, reference)); - symbolsLineReader.read(line1); - symbolsLineReader.read(line2); - symbolsLineReader.read(line3); - symbolsLineReader.read(line4); + assertThat(symbolsLineReader.read(line1)).isEmpty(); + assertThat(symbolsLineReader.read(line2)).isEmpty(); + assertThat(symbolsLineReader.read(line3)).isEmpty(); + assertThat(symbolsLineReader.read(line4)).isEmpty(); assertThat(line1.getSymbols()).isEqualTo(RANGE_LABEL_1 + ",1"); assertThat(line2.getSymbols()).isEmpty(); @@ -256,8 +257,9 @@ public class SymbolsLineReaderTest { SymbolsLineReader symbolsLineReader = newReader(newSymbol(declaration, reference)); - symbolsLineReader.read(line1); - symbolsLineReader.read(line2); + LineReader.ReadError readErrorLine1 = new LineReader.ReadError(SYMBOLS, LINE_1); + assertThat(symbolsLineReader.read(line1)).contains(readErrorLine1); + assertThat(symbolsLineReader.read(line2)).contains(readErrorLine1); assertNoSymbol(); assertThat(logTester.logs(WARN)).isNotEmpty(); @@ -272,8 +274,8 @@ public class SymbolsLineReaderTest { SymbolsLineReader symbolsLineReader = newReader(newSymbol(declaration, reference)); - symbolsLineReader.read(line1); - symbolsLineReader.read(line2); + assertThat(symbolsLineReader.read(line1)).isEmpty(); + assertThat(symbolsLineReader.read(line2)).contains(new LineReader.ReadError(SYMBOLS, LINE_2)); assertThat(line1.hasSymbols()).isTrue(); assertThat(line2.hasSymbols()).isFalse(); @@ -286,7 +288,8 @@ public class SymbolsLineReaderTest { doThrow(RangeOffsetConverter.RangeOffsetConverterException.class).when(rangeOffsetConverter).offsetToString(declaration, LINE_1, DEFAULT_LINE_LENGTH); SymbolsLineReader symbolsLineReader = newReader(newSymbol(declaration, newSingleLineTextRangeWithExpectedLabel(LINE_2, OFFSET_1, OFFSET_3, RANGE_LABEL_2))); - symbolsLineReader.read(line1); + assertThat(symbolsLineReader.read(line1)) + .contains(new LineReader.ReadError(SYMBOLS, LINE_1)); assertThat(logTester.logs(WARN)).containsOnly("Inconsistency detected in Symbols data. Symbols will be ignored for file 'FILE_KEY'"); } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/PersistFileSourcesStepTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/PersistFileSourcesStepTest.java index 7d2b6bd644c..a95d5774636 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/PersistFileSourcesStepTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/PersistFileSourcesStepTest.java @@ -36,8 +36,10 @@ import org.sonar.ce.task.projectanalysis.component.ReportComponent; import org.sonar.ce.task.projectanalysis.component.TreeRootHolderRule; import org.sonar.ce.task.projectanalysis.scm.Changeset; import org.sonar.ce.task.projectanalysis.source.FileSourceDataComputer; +import org.sonar.ce.task.projectanalysis.source.FileSourceDataWarnings; import org.sonar.ce.task.projectanalysis.source.SourceLinesHashRepository; import org.sonar.ce.task.projectanalysis.source.SourceLinesHashRepositoryImpl; +import org.sonar.ce.task.projectanalysis.source.linereader.LineReader; import org.sonar.ce.task.step.ComputationStep; import org.sonar.ce.task.step.TestComputationStepContext; import org.sonar.db.DbClient; @@ -47,10 +49,16 @@ import org.sonar.db.protobuf.DbFileSources; import org.sonar.db.source.FileSourceDto; import org.sonar.db.source.FileSourceDto.Type; import org.sonar.db.source.LineHashVersion; +import org.sonar.scanner.protocol.output.ScannerReport; +import static com.google.common.collect.Lists.newArrayList; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.sonar.ce.task.projectanalysis.source.linereader.LineReader.Data.HIGHLIGHTING; +import static org.sonar.ce.task.projectanalysis.source.linereader.LineReader.Data.SYMBOLS; public class PersistFileSourcesStepTest extends BaseStepTest { @@ -72,8 +80,9 @@ public class PersistFileSourcesStepTest extends BaseStepTest { private SourceLinesHashRepository sourceLinesHashRepository = mock(SourceLinesHashRepository.class); private SourceLinesHashRepositoryImpl.LineHashesComputer lineHashesComputer = mock(SourceLinesHashRepositoryImpl.LineHashesComputer.class); - private FileSourceDataComputer fileSourceDataComputer = mock(FileSourceDataComputer.class); + private FileSourceDataWarnings fileSourceDataWarnings = mock(FileSourceDataWarnings.class); + private DbClient dbClient = dbTester.getDbClient(); private DbSession session = dbTester.getSession(); @@ -83,7 +92,7 @@ public class PersistFileSourcesStepTest extends BaseStepTest { public void setup() { when(system2.now()).thenReturn(NOW); when(sourceLinesHashRepository.getLineHashesComputerToPersist(Mockito.any(Component.class))).thenReturn(lineHashesComputer); - underTest = new PersistFileSourcesStep(dbClient, system2, treeRootHolder, sourceLinesHashRepository, fileSourceDataComputer); + underTest = new PersistFileSourcesStep(dbClient, system2, treeRootHolder, sourceLinesHashRepository, fileSourceDataComputer, fileSourceDataWarnings); initBasicReport(1); } @@ -102,7 +111,7 @@ public class PersistFileSourcesStepTest extends BaseStepTest { DbFileSources.Line.newBuilder().setSource("line2").setLine(2).build() )) .build(); - when(fileSourceDataComputer.compute(fileComponent())).thenReturn(new FileSourceDataComputer.Data(fileSourceData, lineHashes, sourceHash, null)); + when(fileSourceDataComputer.compute(fileComponent(), fileSourceDataWarnings)).thenReturn(new FileSourceDataComputer.Data(fileSourceData, lineHashes, sourceHash, null)); underTest.execute(new TestComputationStepContext()); @@ -123,6 +132,7 @@ public class PersistFileSourcesStepTest extends BaseStepTest { assertThat(data.getLines(0).getSource()).isEqualTo("line1"); assertThat(data.getLines(1).getLine()).isEqualTo(2); assertThat(data.getLines(1).getSource()).isEqualTo("line2"); + verify(fileSourceDataWarnings).commitWarnings(); } @Test @@ -137,6 +147,7 @@ public class PersistFileSourcesStepTest extends BaseStepTest { FileSourceDto fileSourceDto = dbClient.fileSourceDao().selectSourceByFileUuid(session, FILE1_UUID); assertThat(fileSourceDto.getLineHashes()).containsExactly("137f72c3708c6bd0de00a0e5a69c699b", "e6251bcf1a7dc3ba5e7933e325bbe605"); assertThat(fileSourceDto.getSrcHash()).isEqualTo("ee5a58024a155466b43bc559d953e018"); + verify(fileSourceDataWarnings).commitWarnings(); } @Test @@ -156,6 +167,7 @@ public class PersistFileSourcesStepTest extends BaseStepTest { assertThat(dbTester.countRowsOfTable("file_sources")).isEqualTo(1); FileSourceDto fileSourceDto = dbClient.fileSourceDao().selectSourceByFileUuid(session, FILE1_UUID); assertThat(fileSourceDto.getSourceData()).isEqualTo(dbData); + verify(fileSourceDataWarnings).commitWarnings(); } private Component fileComponent() { @@ -179,6 +191,7 @@ public class PersistFileSourcesStepTest extends BaseStepTest { FileSourceDto fileSourceDto = dbClient.fileSourceDao().selectSourceByFileUuid(session, FILE1_UUID); assertThat(fileSourceDto.getSourceData()).isEqualTo(dbData); assertThat(fileSourceDto.getRevision()).isNull(); + verify(fileSourceDataWarnings).commitWarnings(); } @Test @@ -217,6 +230,7 @@ public class PersistFileSourcesStepTest extends BaseStepTest { assertThat(data.getLines(2).getScmAuthor()).isEmpty(); assertThat(data.getLines(2).getScmDate()).isEqualTo(0); assertThat(data.getLines(2).getScmRevision()).isEmpty(); + verify(fileSourceDataWarnings).commitWarnings(); } @Test @@ -236,6 +250,7 @@ public class PersistFileSourcesStepTest extends BaseStepTest { assertThat(data).isEqualTo(dbData); assertThat(data.getLinesList()).hasSize(1); assertThat(data.getLines(0).getHighlighting()).isEqualTo("2,4,a"); + verify(fileSourceDataWarnings).commitWarnings(); } @Test @@ -256,6 +271,7 @@ public class PersistFileSourcesStepTest extends BaseStepTest { assertThat(dbTester.countRowsOfTable("file_sources")).isEqualTo(1); FileSourceDto fileSourceDto = dbClient.fileSourceDao().selectSourceByFileUuid(session, FILE1_UUID); assertThat(fileSourceDto.getSourceData()).isEqualTo(dbData); + verify(fileSourceDataWarnings).commitWarnings(); } @Test @@ -272,6 +288,7 @@ public class PersistFileSourcesStepTest extends BaseStepTest { assertThat(dbTester.countRowsOfTable("file_sources")).isEqualTo(1); FileSourceDto fileSourceDto = dbClient.fileSourceDao().selectSourceByFileUuid(session, FILE1_UUID); assertThat(fileSourceDto.getSourceData()).isEqualTo(dbData); + verify(fileSourceDataWarnings).commitWarnings(); } @Test @@ -283,6 +300,7 @@ public class PersistFileSourcesStepTest extends BaseStepTest { FileSourceDto fileSourceDto = dbClient.fileSourceDao().selectSourceByFileUuid(session, FILE1_UUID); assertThat(fileSourceDto.getRevision()).isEqualTo("rev-1"); + verify(fileSourceDataWarnings).commitWarnings(); } @Test @@ -293,6 +311,7 @@ public class PersistFileSourcesStepTest extends BaseStepTest { FileSourceDto fileSourceDto = dbClient.fileSourceDao().selectSourceByFileUuid(session, FILE1_UUID); assertThat(fileSourceDto.getRevision()).isNull(); + verify(fileSourceDataWarnings).commitWarnings(); } @Test @@ -311,6 +330,7 @@ public class PersistFileSourcesStepTest extends BaseStepTest { assertThat(fileSourceDto.getLineHashes()).isEqualTo(Collections.singletonList("lineHash")); assertThat(fileSourceDto.getCreatedAt()).isEqualTo(PAST); assertThat(fileSourceDto.getUpdatedAt()).isEqualTo(PAST); + verify(fileSourceDataWarnings).commitWarnings(); } @Test @@ -355,6 +375,7 @@ public class PersistFileSourcesStepTest extends BaseStepTest { assertThat(fileSourceDto.getCreatedAt()).isEqualTo(past); assertThat(fileSourceDto.getUpdatedAt()).isEqualTo(NOW); assertThat(fileSourceDto.getRevision()).isEqualTo("rev-1"); + verify(fileSourceDataWarnings).commitWarnings(); } @Test @@ -372,6 +393,7 @@ public class PersistFileSourcesStepTest extends BaseStepTest { assertThat(fileSourceDto.getCreatedAt()).isEqualTo(PAST); assertThat(fileSourceDto.getUpdatedAt()).isEqualTo(NOW); assertThat(fileSourceDto.getSrcHash()).isEqualTo("newSourceHash"); + verify(fileSourceDataWarnings).commitWarnings(); } @Test @@ -396,6 +418,7 @@ public class PersistFileSourcesStepTest extends BaseStepTest { assertThat(fileSourceDto.getCreatedAt()).isEqualTo(PAST); assertThat(fileSourceDto.getUpdatedAt()).isEqualTo(NOW); assertThat(fileSourceDto.getRevision()).isEqualTo("revision"); + verify(fileSourceDataWarnings).commitWarnings(); } private FileSourceDto createDto() { @@ -426,12 +449,12 @@ public class PersistFileSourcesStepTest extends BaseStepTest { private void setComputedData(DbFileSources.Data data, List lineHashes, String sourceHash, Changeset latestChangeWithRevision) { FileSourceDataComputer.Data computedData = new FileSourceDataComputer.Data(data, lineHashes, sourceHash, latestChangeWithRevision); - when(fileSourceDataComputer.compute(fileComponent())).thenReturn(computedData); + when(fileSourceDataComputer.compute(fileComponent(), fileSourceDataWarnings)).thenReturn(computedData); } private void setComputedData(DbFileSources.Data data) { FileSourceDataComputer.Data computedData = new FileSourceDataComputer.Data(data, Collections.emptyList(), "", null); - when(fileSourceDataComputer.compute(fileComponent())).thenReturn(computedData); + when(fileSourceDataComputer.compute(fileComponent(), fileSourceDataWarnings)).thenReturn(computedData); } private void initBasicReport(int numberOfLines) { diff --git a/server/sonar-ce-task/src/test/java/org/sonar/ce/task/step/StepsExplorer.java b/server/sonar-ce-task/src/test/java/org/sonar/ce/task/step/StepsExplorer.java deleted file mode 100644 index d433ff8d120..00000000000 --- a/server/sonar-ce-task/src/test/java/org/sonar/ce/task/step/StepsExplorer.java +++ /dev/null @@ -1,68 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2018 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -package org.sonar.ce.task.step; - -import com.google.common.base.Function; -import com.google.common.base.Predicate; -import java.lang.reflect.Modifier; -import java.util.Set; -import javax.annotation.Nonnull; -import org.reflections.Reflections; - -import static com.google.common.base.Predicates.notNull; -import static com.google.common.collect.FluentIterable.from; - -public class StepsExplorer { - /** - * Compute set of canonical names of classes implementing ComputationStep in the specified package using reflection. - */ - public static Set retrieveStepPackageStepsCanonicalNames(String packageName) { - Reflections reflections = new Reflections(packageName); - - return from(reflections.getSubTypesOf(ComputationStep.class)) - .filter(NotAbstractClass.INSTANCE) - .transform(ClassToCanonicalName.INSTANCE) - // anonymous classes do not have canonical names - .filter(notNull()) - .toSet(); - } - - private enum NotAbstractClass implements Predicate> { - INSTANCE; - - @Override - public boolean apply(Class input) { - return !Modifier.isAbstract(input.getModifiers()); - } - } - - public static Function, String> toCanonicalName() { - return ClassToCanonicalName.INSTANCE; - } - - private enum ClassToCanonicalName implements Function, String> { - INSTANCE; - - @Override - public String apply(@Nonnull Class input) { - return input.getCanonicalName(); - } - } -} -- 2.39.5