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

Completer.getErrors gives quite a lot of false positives #17

Open
najder-k opened this issue Mar 24, 2022 · 1 comment
Open

Completer.getErrors gives quite a lot of false positives #17

najder-k opened this issue Mar 24, 2022 · 1 comment

Comments

@najder-k
Copy link

Example code:

object ScexFlakyValidation {
 private class DefaultJavaScexCompiler(val settings: ScexSettings)
   extends ScexCompiler
     with ScexPresentationCompiler
     with ClassfileReusingScexCompiler
     with TemplateOptimizingScexCompiler
     with CachingScexCompiler // removing this fixes it, but requires a lower count of tests to not crash (1000 is too much)
     with CachingScexPresentationCompiler
     with WeakReferenceWrappingScexCompiler
     with JavaScexCompiler

 def main(args: Array[String]): Unit = {
   val symbolValidator = SymbolValidator(PredefinedAccessSpecs.basicOperations)
   val symbolAttributes = SymbolAttributes(Nil)
   val syntaxValidator = SyntaxValidator.SimpleExpressions
   val utils = NamedSource("test", "")
   val profile = new ExpressionProfile("test", syntaxValidator, symbolValidator, symbolAttributes, "", utils)
   val settings = new ScexSettings
//    settings.noGetterAdapters.value = true //adding this option fixes it too
   val scexCompiler = new DefaultJavaScexCompiler(settings)
   val completer = scexCompiler.getCompleter[SimpleContext[Any], String](profile, template = false)

   def validateExpr(expr: String): Boolean = completer.getErrors(expr).isEmpty

   val invalidExpression = "symbolMissing" //basically any invalid expression works

   val totalCount = 1000
   val fpCount = (0 until totalCount)
     .map(i => invalidExpression + " " * i) //spaces added to avoid hitting the cache
     .count(validateExpr) // <- this is what gives false positives sometimes (completer.getErrors)

   if (fpCount != 0) {
     println(s"There were $fpCount/$totalCount false positives in validation")
   } else {
     println("all good")
   }
 }
}

This code tries to check for errors on the same invalid expression 1000 times (with added spaces to avoid hitting the cache). The number and order of FPs is nondeterministic, and seems to get higher the longer the code runs, e.g. for 1k times it's around 50-70%:

There were 695/1000 false positives in validation

But for 100 times it's between 10-30%:

There were 12/100 false positives in validation

It seems to not happen with either setting the noGetterAdapters = true in ScexSettings, or when using a ScexCompiler without CachingScexCompiler mixed in, even though those seem to be completely unrelated.

Some other findings:

  • it seems it doesn't matter in what way the expression is invalid, unclosed strings, invalid syntax, missing symbols seem to get false positives at the same rate
  • on the same note, random strings can also be used to check the validation (or something like s"missingSymbol$i")
  • while the false positives are nondeterministic between runs, on the same code run, the same expression always gets the same result (might be some caching tho)
  • The specific caches that seem to affect this problem are profileCompilationResultsCache and javaGetterAdaptersCache. With either of them being removed, and the rest of caches still in place, there are no more FPs
  • ScexPresentationCompiler#Completer.workaroundAssertionError looks like the culprit, but it doesn't affect this at all
@anetaporebska
Copy link
Contributor

It seems that there is a problem with com.avsystem.scex.compiler.ScexCompiler.Reporter#errorsBuilder and the fact that errors are cleared both by scex code and the Scala Presentation Compiler code (scala.tools.nsc.interactive.Global#backgroundCompile) by calling reporter.reset(). The latter runs arbitrarily and sometimes interferes with accumulated errors and clears them.

Since I didn't find any direct connection between cache/getterAdapters and errorsBuilder I assume they affect validation time and thus contribute to different results.

Workaround is presented here: #36

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants