bugfix: Try to fix Mill flaky test #5661
Conversation
| _ <- server.server.buildServerPromise.future | ||
| .withTimeout(2, TimeUnit.MINUTES) |
There was a problem hiding this comment.
Doesn't didOpen wait for the buildServerPromise anyway? I though it's more of a when the bsp compiles and publishes things not the connection itself.
There was a problem hiding this comment.
Might be, but I can't figure out where does the test actually hang :/
There was a problem hiding this comment.
Maybe add printing of the bsp trace on error. That could provide some insight.
tgodzik
force-pushed
the
fix-mill-test
branch
2 times, most recently
from
d419f7f to
7a8c6b7
Compare
tests/slow/src/test/scala/tests/mill/MillServerCodeLensSuite.scala
Outdated
Show resolved
Hide resolved
tgodzik
force-pushed
the
fix-mill-test
branch
from
7a8c6b7 to
d55bf95
Compare
tgodzik
force-pushed
the
fix-mill-test
branch
2 times, most recently
from
9e0042d to
177800e
Compare
|
I think I finally found the reason the tests hung and weren't retried. Looks like Mill will sometimes throw an exception which is forwarded to Metals and causes tests to hange, this should make the tests stop hanging and make retries work. |
tgodzik
force-pushed
the
fix-mill-test
branch
from
177800e to
425a2cb
Compare
|
Still seems a mill test failed do you know why? |
Damn, no idea. I need to figure out where it hangs, but I don't see any other possibility. I will add a manual timeout maybe. |
tgodzik
force-pushed
the
fix-mill-test
branch
6 times, most recently
from
f429b30 to
a84b7e1
Compare
|
Looks like finally the approach worked and I run the mill tests 3 times and tests didn't fail. What do you think @kasiaMarek ? |
If it's mill that throws for undisclosed reasons, then it's fine I think. However, I would be great to explain it in a comment and take some steps to try and resolve the underlying problem. Maybe create an issue in |
I opened up an issue in mill com-lihaoyi/mill#2826 I am not 100% that's the reason, but this is the only stack I've seen recently. I added a comment around that test suite, so people might see it's problematic. |
Looks like Mill will sometimes throw an exception which is forwarded to Metals and causes tests to hange, this should make the tests stop hanging and make retries work.
| @@ -1034,6 +1034,7 @@ final case class TestingServer( | |||
| filename: String, | |||
| maxRetries: Int = 4, | |||
| ): Future[List[l.CodeLens]] = { | |||
| Debug.printEnclosing(filename) | |||
There was a problem hiding this comment.
Can you explain to me what does this thing do?
There was a problem hiding this comment.
just prints that this method has been invoked together with the filename.
This is where things like 2023.10.06 10:30:46 INFO tests.BaseLspSuite#initialize come from.
My guess is that the test failed because it takes a while for the build server to connect. Let's see if this helps.