refactor: ensure we're testing the connection with Mill

metals/src/main/resources/millw

@@ -5,7 +5,7 @@
 # If no version is given, it falls back to the value of DEFAULT_MILL_VERSION
 #
 # Project page: https://github.com/lefou/millw
-# Script Version: 0.4.4
+# Script Version: 0.4.5
 #
 # If you want to improve this script, please also contribute your changes back!
 #
@@ -14,7 +14,7 @@
 set -e
 
 if [ -z "${DEFAULT_MILL_VERSION}" ] ; then
-  DEFAULT_MILL_VERSION=0.10.8
+  DEFAULT_MILL_VERSION=0.10.9
 fi
 
 
@@ -169,11 +169,20 @@ if [ -z "$MILL_MAIN_CLI" ] ; then
   MILL_MAIN_CLI="${0}"
 fi
 
+MILL_FIRST_ARG=""
+if [ "$1" = "--bsp" ] || [ "$1" = "-i" ] || [ "$1" = "--interactive" ] || [ "$1" = "--no-server" ] || [ "$1" = "--repl" ] || [ "$1" = "--help" ] ; then
+  # Need to preserve the first position of those listed options
+  MILL_FIRST_ARG=$1
+  shift
+fi
+
 unset MILL_DOWNLOAD_PATH
 unset MILL_OLD_DOWNLOAD_PATH
 unset OLD_MILL
 unset MILL_VERSION
 unset MILL_VERSION_TAG
 unset MILL_REPO_URL
 
-exec "${MILL}" -D "mill.main.cli=${MILL_MAIN_CLI}" "$@"
+# We don't quote MILL_FIRST_ARG on purpose, so we can expand the empty value without quotes
+# shellcheck disable=SC2086
+exec "${MILL}" $MILL_FIRST_ARG -D "mill.main.cli=${MILL_MAIN_CLI}" "$@"

metals/src/main/resources/millw.bat

@@ -5,7 +5,7 @@ rem You can give the required mill version with --mill-version parameter
 rem If no version is given, it falls back to the value of DEFAULT_MILL_VERSION
 rem
 rem Project page: https://github.com/lefou/millw
-rem Script Version: 0.4.4
+rem Script Version: 0.4.5
 rem
 rem If you want to improve this script, please also contribute your changes back!
 rem
@@ -16,25 +16,31 @@ rem but I don't think we need to support them in 2019
 setlocal enabledelayedexpansion
 
 if [!DEFAULT_MILL_VERSION!]==[] (
-    set "DEFAULT_MILL_VERSION=0.10.8"
+    set "DEFAULT_MILL_VERSION=0.10.9"
 )
 
 set "MILL_REPO_URL=https://github.com/com-lihaoyi/mill"
 
 rem %~1% removes surrounding quotes
 if [%~1%]==[--mill-version] (
+  if not [%~2%]==[] (
+    set MILL_VERSION=%~2%
     rem shift command doesn't work within parentheses
-    if not [%~2%]==[] (
-        set MILL_VERSION=%~2%
-        set "STRIP_VERSION_PARAMS=true"
-    ) else (
-        echo You specified --mill-version without a version. 1>&2
-        echo Please provide a version that matches one provided on 1>&2
-        echo %MILL_REPO_URL%/releases 1>&2
-        exit /b 1
-    )
+    set "STRIP_VERSION_PARAMS=true"
+  ) else (
+    echo You specified --mill-version without a version. 1>&2
+    echo Please provide a version that matches one provided on 1>&2
+    echo %MILL_REPO_URL%/releases 1>&2
+    exit /b 1
+  )
 )
 
+if not defined STRIP_VERSION_PARAMS GOTO AfterStripVersionParams
+rem strip the: --mill-version {version}
+shift
+shift
+:AfterStripVersionParams
+
 if [!MILL_VERSION!]==[] (
   if exist .mill-version (
       set /p MILL_VERSION=<.mill-version
@@ -108,19 +114,57 @@ set MILL_DOWNLOAD_PATH=
 set MILL_VERSION=
 set MILL_REPO_URL=
 
-set MILL_PARAMS=%*
-
 if [!MILL_MAIN_CLI!]==[] (
     set "MILL_MAIN_CLI=%0"
 )
 
-if defined STRIP_VERSION_PARAMS (
+rem Need to preserve the first position of those listed options
+set MILL_FIRST_ARG=
+if [%~1%]==[--bsp] (
+  set MILL_FIRST_ARG=%1%
+) else (
+  if [%~1%]==[-i] (
+    set MILL_FIRST_ARG=%1%
+  ) else (
+    if [%~1%]==[--interactive] (
+      set MILL_FIRST_ARG=%1%
+    ) else (
+      if [%~1%]==[--no-server] (
+        set MILL_FIRST_ARG=%1%
+      ) else (
+        if [%~1%]==[--repl] (
+          set MILL_FIRST_ARG=%1%
+        ) else (
+          if [%~1%]==[--help] (
+            set MILL_FIRST_ARG=%1%
+          )
+        )
+      )
+    )
+  )
+)
+
+set "MILL_PARAMS=%*%"
+
+if not [!MILL_FIRST_ARG!]==[] (
+  if defined STRIP_VERSION_PARAMS (
+    for /f "tokens=1-3*" %%a in ("%*") do (
+        set "MILL_PARAMS=%%d"
+    )
+  ) else (
+    for /f "tokens=1*" %%a in ("%*") do (
+      set "MILL_PARAMS=%%b"
+    )
+  )
+) else (
+  if defined STRIP_VERSION_PARAMS (
     for /f "tokens=1-2*" %%a in ("%*") do (
         rem strip %%a - It's the "--mill-version" option.
         rem strip %%b - it's the version number that comes after the option.
         rem keep  %%c - It's the remaining options.
-        set MILL_PARAMS=%%c
+        set "MILL_PARAMS=%%c"
     )
+  )
 )
 
-"%MILL%" -D "mill.main.cli=%MILL_MAIN_CLI%" %MILL_PARAMS%
+"%MILL%" %MILL_FIRST_ARG% -D "mill.main.cli=%MILL_MAIN_CLI%" %MILL_PARAMS%

tests/slow/src/test/scala/tests/mill/MillServerSuite.scala

@@ -1,5 +1,7 @@
 package tests.mill
 
+import scala.concurrent.Promise
+
 import scala.meta.internal.builds.MillBuildTool
 import scala.meta.internal.builds.MillDigest
 import scala.meta.internal.metals.Messages
@@ -13,7 +15,7 @@ import tests.MillBuildLayout
 import tests.MillServerInitializer
 
 /**
- * Basic suite to ensure that a connection to sbt server can be made.
+ * Basic suite to ensure that a connection to a Mill server can be made.
  */
 class MillServerSuite
     extends BaseImportSuite("mill-server", MillServerInitializer) {
@@ -49,25 +51,40 @@ class MillServerSuite
     }
   }
 
-  test("generate") {
-    def millBspConfig = workspace.resolve(".bsp/mill-bsp.json")
-    cleanWorkspace()
-    writeLayout(MillBuildLayout("", V.scala213, supportedBspVersion))
-    for {
-      _ <- server.initialize()
-      _ <- server.initialized()
-      _ = assertNoDiff(
-        client.workspaceMessageRequests,
-        // Project has no .bloop directory so user is asked to "import via bloop"
-        // since bloop is still the default
-        importBuildMessage,
-      )
-      _ = client.messageRequests.clear() // restart
-      _ = assert(!millBspConfig.exists)
-      // At this point, we want to use mill-bsp server, so create the mill-bsp.json file.
-      _ <- server.executeCommand(ServerCommands.GenerateBspConfig)
-    } yield {
-      assert(millBspConfig.exists)
+  val versionsToTest: List[String] =
+    List("0.10.0", "0.10.8", supportedBspVersion)
+
+  versionsToTest.foreach(testGenerationAndConnection)
+
+  private def testGenerationAndConnection(version: String) = {
+    test(s"generate-and-connect-$version") {
+      def millBspConfig = workspace.resolve(".bsp/mill-bsp.json")
+      cleanWorkspace()
+      writeLayout(MillBuildLayout("", V.scala213, version))
+      for {
+        _ <- server.initialize()
+        _ <- server.initialized()
+        _ = assertNoDiff(
+          client.workspaceMessageRequests,
+          // Project has no .bloop directory so user is asked to "import via bloop"
+          // since bloop is still the default
+          importBuildMessage,
+        )
+        _ = client.messageRequests.clear() // restart
+        _ = assert(!millBspConfig.exists)
+        // This is a little hacky but up above this promise is suceeded already, so down
+        // below it won't wait until it reconnects to Mill like we want, so we set it back
+        // and then it will be completed after the BSP config generation and the server
+        // connects.
+        _ = server.server.buildServerPromise = Promise()
+        // At this point, we want to use mill-bsp server, so create the mill-bsp.json file.
+        _ <- server.executeCommand(ServerCommands.GenerateBspConfig)
+        // We need to wait a bit just to ensure the connection is made
+        _ <- server.server.buildServerPromise.future
+      } yield {
+        assert(millBspConfig.exists)
+        server.assertBuildServerConnection()
+      }
     }
   }
 }