Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement retry logic for dealing with compiler errors #946

Merged
merged 27 commits into from
Oct 23, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
538efe4
First version of the retry logic for dealing with compiler errors
MartinWelgemoed Aug 25, 2021
6b871b2
Removed somewhat hacky second file id for mutants
MartinWelgemoed Aug 27, 2021
8cd27e6
Restructured retry flow for compiler errors
MartinWelgemoed Aug 27, 2021
4a338bc
Removed parse[Stat], because it always deals with complete source files
MartinWelgemoed Aug 27, 2021
e03136a
Improved code style in MatchBuilder
MartinWelgemoed Aug 27, 2021
71897b0
Now only tracks mutant case statements after a compiler error
MartinWelgemoed Aug 27, 2021
6751f30
Fixed compiler error in Maven test
MartinWelgemoed Aug 27, 2021
5b5bb37
Fixed more maven compiler errors
MartinWelgemoed Sep 1, 2021
d2bf7a8
Fixed scalafmt issues on maven plugin
MartinWelgemoed Sep 1, 2021
b544805
Using IO.pure() instead of the apply
MartinWelgemoed Sep 27, 2021
9eb0f2a
Using CompileError as MutantRunResult and renamed the model class to …
MartinWelgemoed Sep 27, 2021
fae89bf
Using UnableToFixCompilerErrorsException instead of RuntimeException
MartinWelgemoed Sep 28, 2021
c77d70d
Removed unneeded fatory logic for the Mutator
MartinWelgemoed Sep 28, 2021
161a8d9
Moved mutation rollback logic into its own class
MartinWelgemoed Sep 28, 2021
36c46a5
Merge branch 'master' into retry_compiler_errors
MartinWelgemoed Sep 29, 2021
465ee74
Fixed merge issue
MartinWelgemoed Sep 30, 2021
10e9458
Cleanup and tests for rollback logic
MartinWelgemoed Sep 30, 2021
18202be
Added test and performance improvement in the RollbackHandler
MartinWelgemoed Sep 30, 2021
ee05a0b
Added sbt scripted test for compiler errors
MartinWelgemoed Oct 1, 2021
568dfee
Fixed scalafmt in test project
MartinWelgemoed Oct 1, 2021
5af8a10
Fixed Maven project using old CompilerError class
MartinWelgemoed Oct 1, 2021
322a84b
Cleanup and better logging for mutant rollbacks
MartinWelgemoed Oct 1, 2021
7e1784f
No longer comparing files as strings
MartinWelgemoed Oct 1, 2021
45d8cef
Scalafmt
MartinWelgemoed Oct 1, 2021
537dd49
Using case statements after the StatementTransformer to search for mu…
MartinWelgemoed Oct 1, 2021
5439ac1
Fixed name in test
MartinWelgemoed Oct 1, 2021
0a9fd4d
small changes
hugo-vrijswijk Oct 16, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class ProcessTestRunner(command: Command, processRunner: ProcessRunner, tmpDir:
}

def runMutant(mutant: Mutant): IO[MutantRunResult] = {
val id = mutant.id
val id = mutant.id.globalId
processRunner(command, tmpDir, ("ACTIVE_MUTATION", id.toString)).map {
case Success(0) => Survived(mutant)
case Success(_) => Killed(mutant)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class ProcessTestRunnerTest extends Stryker4sIOSuite with MockitoIOSuite with Lo

it("should return a Survived mutant on an exitcode 0 process") {
val testProcessRunner = TestProcessRunner(Success(0))
val mutant = Mutant(0, q"4", q"5", EmptyString)
val mutant = Mutant(MutantId(0), q"4", q"5", EmptyString)
processTestRunner(testProcessRunner).runMutant(mutant).asserting { result =>
result shouldBe a[Survived]
testProcessRunner.timesCalled.next() should equal(1)
Expand All @@ -31,7 +31,7 @@ class ProcessTestRunnerTest extends Stryker4sIOSuite with MockitoIOSuite with Lo

it("should return a Killed mutant on an exitcode 1 process") {
val testProcessRunner = TestProcessRunner(Success(1))
val mutant = Mutant(0, q"4", q"5", EmptyString)
val mutant = Mutant(MutantId(0), q"4", q"5", EmptyString)
processTestRunner(testProcessRunner).runMutant(mutant).asserting { result =>
result shouldBe a[Killed]
testProcessRunner.timesCalled.next() should equal(1)
Expand All @@ -41,7 +41,7 @@ class ProcessTestRunnerTest extends Stryker4sIOSuite with MockitoIOSuite with Lo
it("should return a TimedOut mutant on a TimedOut process") {
val exception = new TimeoutException("Test")
val testProcessRunner = TestProcessRunner(Failure(exception))
val mutant = Mutant(0, q"4", q"5", EmptyString)
val mutant = Mutant(MutantId(0), q"4", q"5", EmptyString)
processTestRunner(testProcessRunner).runMutant(mutant).asserting { result =>
result shouldBe a[TimedOut]
testProcessRunner.timesCalled.next() should equal(1)
Expand Down
38 changes: 32 additions & 6 deletions core/src/main/scala/stryker4s/Stryker4s.scala
Original file line number Diff line number Diff line change
@@ -1,21 +1,47 @@
package stryker4s

import cats.effect.IO
import mutationtesting.MetricsResult
import stryker4s.config.Config
import stryker4s.files.MutatesFileResolver
import stryker4s.model.MutantId
import stryker4s.mutants.Mutator
import stryker4s.run.MutantRunner
import stryker4s.run.threshold.{ScoreStatus, ThresholdChecker}

class Stryker4s(fileSource: MutatesFileResolver, mutator: Mutator, runner: MutantRunner)(implicit config: Config) {
case class CompileError(msg: String, path: String, line: Integer) {
hugo-vrijswijk marked this conversation as resolved.
Show resolved Hide resolved
override def toString: String = s"$path:L$line: '$msg'"
}
case class MutationCompilationFailed(errors: Seq[CompileError]) extends RuntimeException(s"Compilation failed: $errors")

class Stryker4s(fileSource: MutatesFileResolver, mutatorFactory: () => Mutator, runner: MutantRunner)(implicit
hugo-vrijswijk marked this conversation as resolved.
Show resolved Hide resolved
config: Config
) {

def run(): IO[ScoreStatus] = {
val filesToMutate = fileSource.files
runWithRetry(List.empty)
.map(metrics => ThresholdChecker.determineScoreStatus(metrics.mutationScore))
}

//Retries the run, after removing the non-compiling mutants
def runWithRetry(nonCompilingMutants: Seq[MutantId]): IO[MetricsResult] = {
//Recreate the mutator from the factory, otherwise the second run's ids will not start at 0
val mutator = mutatorFactory()
val filesToMutate = fileSource.files
for {
mutatedFiles <- mutator.mutate(filesToMutate)
metrics <- runner(mutatedFiles)
scoreStatus = ThresholdChecker.determineScoreStatus(metrics.mutationScore)
} yield scoreStatus
mutatedFiles <- mutator.mutate(filesToMutate, nonCompilingMutants)
metrics <- runner(mutatedFiles, nonCompilingMutants).handleErrorWith {
hugo-vrijswijk marked this conversation as resolved.
Show resolved Hide resolved
//If a compiler error occurs, retry once without the lines that gave an error
case MutationCompilationFailed(errs) if nonCompilingMutants.isEmpty =>
val nonCompilingMutations = mutator.errorsToIds(errs, mutatedFiles)
if (nonCompilingMutations.nonEmpty) {
runWithRetry(nonCompilingMutations)
} else {
IO.raiseError(new RuntimeException("Unable to see which mutations caused the compiler failure"))
}
//Something else went wrong vOv
case e => IO.raiseError(e)
}
} yield metrics
}
}
15 changes: 14 additions & 1 deletion core/src/main/scala/stryker4s/model/Mutant.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,17 @@ import scala.meta.{Term, Tree}

import stryker4s.extension.mutationtype.Mutation

final case class Mutant(id: Int, original: Term, mutated: Term, mutationType: Mutation[_ <: Tree])
//The globalId is used in mutation switching and generating reports, but it varies between runs
hugo-vrijswijk marked this conversation as resolved.
Show resolved Hide resolved
//The file and idInFile is stable, and used for finding compiler errors
case class MutantId(globalId: Int, file: String, idInFile: Int) {
def sameMutation(otherMutId: MutantId): Boolean = {
otherMutId.idInFile == this.idInFile && otherMutId.file == this.file
}
}

object MutantId {
//Initially mutants are created with just a globalId, the file information is added later on
def apply(globalId: Int): MutantId = new MutantId(globalId, "", -1)
}

final case class Mutant(id: MutantId, original: Term, mutated: Term, mutationType: Mutation[_ <: Tree])
2 changes: 2 additions & 0 deletions core/src/main/scala/stryker4s/model/MutantRunResult.scala
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,5 @@ final case class NoCoverage(mutant: Mutant, description: Option[String] = None)
final case class Error(mutant: Mutant, description: Option[String] = None) extends MutantRunResult

final case class Ignored(mutant: Mutant, description: Option[String] = None) extends MutantRunResult

final case class CompilerError(mutant: Mutant, description: Option[String] = None) extends MutantRunResult
37 changes: 35 additions & 2 deletions core/src/main/scala/stryker4s/model/MutatedFile.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,39 @@ package stryker4s.model

import fs2.io.file.Path

import scala.meta.Tree
import scala.meta._

final case class MutatedFile(fileOrigin: Path, tree: Tree, mutants: Seq[Mutant], excludedMutants: Int)
final case class MutatedFile(
fileOrigin: Path,
tree: Tree,
mutants: Seq[Mutant],
mutationStatements: Seq[(MutantId, Tree)],
excludedMutants: Int
) {

def mutatedSource: String = {
tree.syntax
}

//Returns a map of line numbers to the mutant on that line
//Contains only mutated lines, and the same mutant can stretch over multiple multiple lines
//This logic is not very fast, because it has to search the entire tree, that's why it's lazy
lazy val mutantLineNumbers: Map[Int, MutantId] = {
hugo-vrijswijk marked this conversation as resolved.
Show resolved Hide resolved
val statementToMutIdMap = mutationStatements.map { case (mutantId, mutationStatement) =>
mutationStatement.structure -> mutantId
}.toMap

mutatedSource
.parse[Stat] //Parse as a standalone statement, used in unit tests and conceivable in some real code
hugo-vrijswijk marked this conversation as resolved.
Show resolved Hide resolved
.orElse(mutatedSource.parse[Source]) //Parse as a complete scala source file
.get //If both failed something has gone very badly wrong, give up
.collect {
case node if statementToMutIdMap.contains(node.structure) =>
val mutId = statementToMutIdMap(node.structure)
//+1 because scalameta uses zero-indexed line numbers
(node.pos.startLine to node.pos.endLine).map(i => i + 1 -> mutId)
}
.flatten
.toMap
}
}
28 changes: 20 additions & 8 deletions core/src/main/scala/stryker4s/mutants/Mutator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,28 +4,40 @@ import cats.effect.IO
import cats.syntax.functor._
import fs2.Stream
import fs2.io.file.Path
import stryker4s.CompileError
import stryker4s.config.Config
import stryker4s.extension.StreamExtensions._
import stryker4s.log.Logger
import stryker4s.model.{MutatedFile, MutationsInSource, SourceTransformations}
import stryker4s.model.{MutantId, MutatedFile, MutationsInSource, SourceTransformations}
import stryker4s.mutants.applymutants.{MatchBuilder, StatementTransformer}
import stryker4s.mutants.findmutants.MutantFinder

import scala.meta.Tree

class Mutator(mutantFinder: MutantFinder, transformer: StatementTransformer, matchBuilder: MatchBuilder)(implicit
config: Config,
log: Logger
) {

def mutate(files: Stream[IO, Path]): IO[Seq[MutatedFile]] = {
//Given compiler errors, return the mutants that caused it
def errorsToIds(compileError: Seq[CompileError], files: Seq[MutatedFile]): Seq[MutantId] = {
compileError.flatMap { err =>
files
//Find the file that the compiler error came from
.find(_.fileOrigin.toString.endsWith(err.path))
//Find the mutant case statement that cased the compiler error
.flatMap(file => file.mutantLineNumbers.get(err.line))
}
}

def mutate(files: Stream[IO, Path], nonCompilingMutants: Seq[MutantId] = Seq.empty): IO[Seq[MutatedFile]] = {
files
.parEvalMapUnordered(config.concurrency)(p => findMutants(p).tupleLeft(p))
.map { case (file, mutationsInSource) =>
val transformed = transformStatements(mutationsInSource)
val builtTree = buildMatches(transformed)
val validMutants =
mutationsInSource.mutants.filterNot(mut => nonCompilingMutants.exists(_.sameMutation(mut.id)))
val transformed = transformStatements(mutationsInSource.copy(mutants = validMutants))
val (builtTree, mutations) = buildMatches(transformed)

MutatedFile(file, builtTree, mutationsInSource.mutants, mutationsInSource.excluded)
MutatedFile(file, builtTree, mutationsInSource.mutants, mutations, mutationsInSource.excluded)
}
.filterNot(mutatedFile => mutatedFile.mutants.isEmpty && mutatedFile.excludedMutants == 0)
.compile
Expand All @@ -44,7 +56,7 @@ class Mutator(mutantFinder: MutantFinder, transformer: StatementTransformer, mat

/** Step 3: Build pattern matches from transformed trees
*/
private def buildMatches(transformedMutantsInSource: SourceTransformations): Tree =
private def buildMatches(transformedMutantsInSource: SourceTransformations) =
matchBuilder.buildNewSource(transformedMutantsInSource)

private def logMutationResult(mutatedFiles: Iterable[MutatedFile]): IO[Unit] = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class CoverageMatchBuilder(mutationContext: ActiveMutationContext)(implicit log:

// sbt-stryker4s-testrunner matches on Int instead of Option[Int]
override def mutantToCase(mutant: Mutant): Case =
super.buildCase(mutant.mutated, p"${mutant.id}")
super.buildCase(mutant.mutated, p"${mutant.id.globalId}")

override def defaultCase(transformedMutant: TransformedMutants): Case =
withCoverage(super.defaultCase(transformedMutant), transformedMutant.mutantStatements)
Expand All @@ -20,7 +20,7 @@ class CoverageMatchBuilder(mutationContext: ActiveMutationContext)(implicit log:
* switch. `coverMutant` always returns true
*/
private def withCoverage(caze: Case, mutants: List[Mutant]): Case = {
val coverageCond = q"_root_.stryker4s.coverage.coverMutant(..${mutants.map(_.id).map(Lit.Int(_))})"
val coverageCond = q"_root_.stryker4s.coverage.coverMutant(..${mutants.map(_.id.globalId).map(Lit.Int(_))})"
caze.copy(cond = Some(coverageCond))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,50 +3,52 @@ package stryker4s.mutants.applymutants
import stryker4s.extension.TreeExtensions.{IsEqualExtension, TransformOnceExtension}
import stryker4s.extension.exception.UnableToBuildPatternMatchException
import stryker4s.log.Logger
import stryker4s.model.{Mutant, SourceTransformations, TransformedMutants}
import stryker4s.model.{Mutant, MutantId, SourceTransformations, TransformedMutants}
import stryker4s.mutants.applymutants.ActiveMutationContext.ActiveMutationContext

import scala.meta._
import scala.util.{Failure, Success}

class MatchBuilder(mutationContext: ActiveMutationContext)(implicit log: Logger) {
def buildNewSource(transformedStatements: SourceTransformations): Tree = {
def buildNewSource(transformedStatements: SourceTransformations): (Tree, Seq[(MutantId, Case)]) = {
val source = transformedStatements.source

groupTransformedStatements(transformedStatements).foldLeft(source: Tree) { (rest, mutants) =>
hugo-vrijswijk marked this conversation as resolved.
Show resolved Hide resolved
val origStatement = mutants.originalStatement
groupTransformedStatements(transformedStatements).foldLeft((source, Seq.empty): (Tree, Seq[(MutantId, Case)])) {
(rest, mutants) =>
val origStatement = mutants.originalStatement

var isTransformed = false
rest transformOnce {
case found if found.isEqual(origStatement) && found.pos == origStatement.pos =>
isTransformed = true
buildMatch(mutants)
} match {
case Success(value) if isTransformed => value
case Success(value) =>
log.warn(
s"Failed to add mutation(s) ${mutants.mutantStatements.map(_.id).mkString(", ")} to new mutated code"
)
log.warn(
s"The code that failed to mutate was: [$origStatement] at ${origStatement.pos.input}:${origStatement.pos.startLine + 1}:${origStatement.pos.startColumn + 1}"
)
log.warn("This mutation will likely show up as Survived")
log.warn(
"Please open an issue on github with sample code of the mutation that failed: https://github.com/stryker-mutator/stryker4s/issues/new"
)
value
case Failure(exception) =>
log.error(s"Failed to construct pattern match: original statement [$origStatement]")
log.error(s"Failed mutation(s) ${mutants.mutantStatements.mkString(",")}.")
log.error(
s"at ${origStatement.pos.input}:${origStatement.pos.startLine + 1}:${origStatement.pos.startColumn + 1}"
)
log.error("This is likely an issue on Stryker4s's end, please enable debug logging and restart Stryker4s.")
log.debug("Please open an issue on github: https://github.com/stryker-mutator/stryker4s/issues/new")
log.debug("Please be so kind to copy the stacktrace into the issue", exception)
var isTransformed = false
rest._1 transformOnce {
case found if found.isEqual(origStatement) && found.pos == origStatement.pos =>
isTransformed = true
buildMatch(mutants)
} match {
case Success(value) if isTransformed =>
(value, rest._2 ++ mutants.mutantStatements.map(mut => (mut.id, mutantToCase(mut))))
case Success(value) =>
log.warn(
s"Failed to add mutation(s) ${mutants.mutantStatements.map(_.id.globalId).mkString(", ")} to new mutated code"
)
log.warn(
s"The code that failed to mutate was: [$origStatement] at ${origStatement.pos.input}:${origStatement.pos.startLine + 1}:${origStatement.pos.startColumn + 1}"
)
log.warn("This mutation will likely show up as Survived")
log.warn(
"Please open an issue on github with sample code of the mutation that failed: https://github.com/stryker-mutator/stryker4s/issues/new"
)
(value, rest._2)
case Failure(exception) =>
log.error(s"Failed to construct pattern match: original statement [$origStatement]")
log.error(s"Failed mutation(s) ${mutants.mutantStatements.mkString(",")}.")
log.error(
s"at ${origStatement.pos.input}:${origStatement.pos.startLine + 1}:${origStatement.pos.startColumn + 1}"
)
log.error("This is likely an issue on Stryker4s's end, please enable debug logging and restart Stryker4s.")
log.debug("Please open an issue on github: https://github.com/stryker-mutator/stryker4s/issues/new")
log.debug("Please be so kind to copy the stacktrace into the issue", exception)

throw UnableToBuildPatternMatchException()
}
throw UnableToBuildPatternMatchException()
}
}
}

Expand All @@ -57,7 +59,7 @@ class MatchBuilder(mutationContext: ActiveMutationContext)(implicit log: Logger)
}

protected def mutantToCase(mutant: Mutant): Case =
buildCase(mutant.mutated, p"Some(${mutant.id})")
buildCase(mutant.mutated, p"Some(${mutant.id.globalId})")

protected def defaultCase(transformedMutant: TransformedMutants): Case =
buildCase(transformedMutant.originalStatement, p"_")
Expand All @@ -72,6 +74,8 @@ class MatchBuilder(mutationContext: ActiveMutationContext)(implicit log: Logger)
}
.map { case (originalStatement, mutants) => TransformedMutants(originalStatement, mutants.toList) }
.toSeq
.sortBy(_.mutantStatements.head.id) // Should be sorted so tree transformations are applied in order of discovery
.sortBy(
_.mutantStatements.head.id.globalId
) // Should be sorted so tree transformations are applied in order of discovery
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class StatementTransformer {
.groupBy(mutant => mutant.original)
.map { case (original, mutants) => transformMutant(original, mutants) }
.toSeq
.sortBy(_.mutantStatements.map(_.id).max)
.sortBy(_.mutantStatements.map(_.id.globalId).max)

SourceTransformations(source, transformedMutants)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import fs2.io.file.Path
import stryker4s.config.Config
import stryker4s.extension.FileExtensions._
import stryker4s.log.Logger
import stryker4s.model.{Mutant, MutationExcluded, MutationsInSource, RegexParseError}
import stryker4s.model.{Mutant, MutantId, MutationExcluded, MutationsInSource, RegexParseError}

import scala.meta.parsers.XtensionParseInputLike
import scala.meta.{Dialect, Parsed, Source}
Expand All @@ -15,7 +15,15 @@ class MutantFinder(matcher: MutantMatcher)(implicit config: Config, log: Logger)
def mutantsInFile(filePath: Path): IO[MutationsInSource] = for {
parsedSource <- parseFile(filePath)
(included, excluded) <- IO(findMutants(parsedSource))
} yield MutationsInSource(parsedSource, included, excluded)
} yield MutationsInSource(parsedSource, addFileInfo(filePath.toString, included), excluded)

def addFileInfo(filePath: String, mutants: Seq[Mutant]): Seq[Mutant] = {
hugo-vrijswijk marked this conversation as resolved.
Show resolved Hide resolved
mutants.zipWithIndex.map { case (mut, numInFile) =>
//Assign the file and the index of the mutation in the file to the MutantId
//This is used later to trace potential compiler errors back to the mutation
mut.copy(id = MutantId(mut.id.globalId, filePath, numInFile))
}
}

def findMutants(source: Source): (Seq[Mutant], Int) = {
val (ignored, included) = source.collect(matcher.allMatchers).flatten.partitionEither(identity)
Expand Down
Loading