FormatTests intermittently (but frequently) fails in the Scala 2 community build

[SethTisue]

if something fails 5% of the time it's not a real concern, but lately FormatTests has been failing quite often, enough to be an annoyance. it might be an annoyance for casual contributors as well?

I don't know when it started exactly, within the last couple months I think?

example failure log: https://scala-ci.typesafe.com/job/scala-2.13.x-jdk11-integrate-community-build/4760/artifact/logs/scalafmt-build.log

[scalafmt] �[91m==> X �[0m�[91morg.scalafmt.FormatTests�[0m.�[91mscala3/OptionalBraces.stat:1411: rewrite to new syntax, wildcard      |�[0m  �[90m0.028s�[0m munit.ComparisonFailException: /home/jenkins/workspace/scala-2.13.x-jdk11-integrate-community-build/target-0.9.20/project-builds/scalafmt-e124d7c934a26325616e1db756c2da4eb7d1adf3/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat:1411
[scalafmt] 1410:}
[scalafmt] �[7m1411:<<< rewrite to new syntax, wildcard�[0m
[scalafmt] 1412:rewrite.scala3.convertToNewSyntax = true
[scalafmt] values are not the same
[scalafmt] �[1m=> Obtained�[0m
[scalafmt] object a:
[scalafmt]    def foo(a: Foo[_]): Unit = ???
[scalafmt] 
[scalafmt] �[1m=> Diff�[0m (�[91m- obtained�[0m, �[92m+ expected�[0m)
[scalafmt]  object a:
[scalafmt] �[91m-   def foo(a: Foo[_]): Unit = ???�[0m
[scalafmt] �[92m+   def foo(a: Foo[?]): Unit = ???�[0m
[scalafmt] �[90m    at �[0m�[90mmunit.FunSuite.assertEquals�[0m�[90m(�[0m�[90mFunSuite.scala�[0m:�[90m11�[0m�[90m)�[0m
[scalafmt] �[1m    at �[0m�[1morg.scalafmt.FormatTests.run�[0m�[1m(�[0m�[1mFormatTests.scala�[0m:�[1m86�[0m�[1m)�[0m
[scalafmt] �[1m    at �[0m�[1morg.scalafmt.FormatTests.$anonfun$new$3�[0m�[1m(�[0m�[1mFormatTests.scala�[0m:�[1m33�[0m�[1m)�[0m
[scalafmt] �[1m    at �[0m�[1morg.scalafmt.FormatTests.$anonfun$new$3$adapted�[0m�[1m(�[0m�[1mFormatTests.scala�[0m:�[1m33�[0m�[1m)�[0m
[scalafmt] �[1m    at �[0m�[1morg.scalafmt.util.CanRunTests.$anonfun$runTest$2�[0m�[1m(�[0m�[1mCanRunTests.scala�[0m:�[1m19�[0m�[1m)�[0m

what looks to me like the same failure occurs in OptionalnalBraces but also in multiple variants of it such as OptionnalBraces_keep

for now I'm going to disable FormatTests in the community build, which will eliminate the problem from my end, but also reduce the chances of us detecting Scala regressions before we release them and of you getting early notification of actual, non-intermittent failures due to Scala changes

[kitbellew]

@SethTisue where is the build defined? it seems that it checks out scalafmt from source but then i cannot see where it gets scalameta (the parser) from, the precise version that scalafmt was built with (it's not really compatible with any other version of the parser, because it might not expect the parsed trees in other versions correctly).

[SethTisue]

the scalafmt config is https://github.com/scala/community-build/blob/2.13.x/core/scalafmt.conf

the scalameta config is https://github.com/scala/community-build/blob/2.13.x/core/scalameta.conf

maybe the scalameta version I'm using is too old? scalafix is on 4.8.4, but you're on 4.8.10. I could ask the scalafix people if they can get on 4.8.10

[SethTisue]

it's not really compatible with any other version of the parser, because it might not expect the parsed trees in other versions correctly

that wouldn't explain why the tests sometimes pass, though, right? if they consistently failed in the same way, that would be easy to understand. but it's intermittent.

do you think the apparently nondeterministic behavior originates in Scalameta, or might it be in this repo? seems like this stuff ought to behave deterministically

[SethTisue]

Ah, I see that you're involved with Scalafix's Scalameta upgrade at scalacenter/scalafix#1857 — maybe once that goes through we could return to this?

[SethTisue]

(by the way, I'm sorry if we've discussed this before, and I'm not remembering?)

[kitbellew]

1. nope, i don't think we have discussed.
2. you are right, i don't know why it's intermittent.

i can try to make this particular check more resilient, perhaps. it looks for a wildcard type parameter (a tree) and replaces underscore with question mark if the dialect allows it. so either the dialect is sometimes wrong (why would it be?), or the parsed tree is sometimes different (also should be the same if the scalameta being downloaded is always the same)...

there's no chance that the local copy of scalameta git repo is somehow cached and might be on a different ref?

[SethTisue]

there's no chance that the local copy of scalameta git repo is somehow cached and might be on a different ref?

dbuild reliably uses the commit specified. it's definitely using https://github.com/scalacommunitybuild/scalameta/commits/community-build-2.13 — which is the v4.8.3 tag plus my one small commit

[SethTisue]

I suppose one possibility is that the nondeterminism originates in the Scala compiler itself. seems unlikely, but we can't rule it out a priori

[kitbellew]

@SethTisue I am proposing to add another test which validates the full parsed tree, to see what's going on. That would mean re-enabling it on your end.

[SethTisue]

@kitbellew tried it out, failure log is here: https://scala-ci.typesafe.com/job/scala-2.13.x-jdk17-integrate-community-build/1385/artifact/logs/scalafmt-build.log

[kitbellew]

@kitbellew tried it out, failure log is here: https://scala-ci.typesafe.com/job/scala-2.13.x-jdk17-integrate-community-build/1385/artifact/logs/scalafmt-build.log

Different failures this time. scalafmt tests infix with fewer braces that only the most recent scalameta supports, but the problem you had reported earlier (and my Custom test) is fine.

Looks like scalafmt must really be tested with the right version of scalameta, either by downloading the binary release or checking out the source.

[SethTisue]

@kitbellew sorry, I got confused — scala/community-build#1700 isn't merged yet. waiting on Metals for that one to move forward. so once that's done I can try scalafmt again.

[SethTisue]

scala/community-build#1700 is now merged. I've re-enabled FormatTests in a separate commit (scala/community-build@163abe9) and we'll see how it goes — I did get a green run locally

[SethTisue]

haven't seen any failures in a while 👍