Fix windows ci (#851)

* Reenable CI on Windows

* Replaced hard coded paths with calls to Path.join

* Testing the approach of updating expectations manually

* Fixed test

also applied a fix from another branch to see if the tests pass

* Fixed straggler path that breaks in windows

* Changed error message so we can see what's going on in windows

* Normalized parent and child to ensure subdirectory? succeeds

* Change to figure out why inputs aren't matching

* formatted

* One more time...

* Converted the uri to a path to get system separators

* Trying with paths again

* normalized both sides...

* fixed formatting

* fix formatter tests on windows

* rename function

* remove not needed functions

---------

Co-authored-by: Steve Cohen <scohen@scohen.org>

Author: lukaszsamson

.github/workflows/ci.yml

@@ -49,50 +49,48 @@ jobs:
           mix deps.get --only test
       - run: mix test
 
-  # TODO reenable when https://github.com/erlef/setup-beam/issues/167 is fixed
-  # currently windows builds are failing with 403s on setup_beam action
-  # mix_test_windows:
-  #   name: mix test windows (Elixir ${{matrix.elixir}} | Erlang/OTP ${{matrix.otp}})
-  #   runs-on: windows-2019
-  #   strategy:
-  #     fail-fast: false
-  #     matrix:
-  #       include:
-  #         - elixir: 1.12.x
-  #           otp: 22.x
-  #         - elixir: 1.12.x
-  #           otp: 23.x
-  #         - elixir: 1.13.x
-  #           otp: 22.x
-  #         - elixir: 1.13.x
-  #           otp: 23.x
-  #         - elixir: 1.13.x
-  #           otp: 24.x
-  #         - elixir: 1.13.x
-  #           otp: 25.x
-  #         - elixir: 1.14.x
-  #           otp: 23.x
-  #         - elixir: 1.14.x
-  #           otp: 24.x
-  #         - elixir: 1.14.x
-  #           otp: 25.x
-  #   env:
-  #     MIX_ENV: test
-  #   steps:
-  #     - name: Set git to use original line ending
-  #       run: |
-  #         git config --global core.autocrlf false
-  #     - uses: actions/checkout@v3
-  #     - uses: erlef/setup-beam@v1
-  #       with:
-  #         otp-version: ${{matrix.otp}}
-  #         elixir-version: ${{matrix.elixir}}
-  #     - name: Install Dependencies
-  #       run: |
-  #         mix local.hex --force
-  #         mix local.rebar --force
-  #         mix deps.get --only test
-  #     - run: mix test
+  mix_test_windows:
+    name: mix test windows (Elixir ${{matrix.elixir}} | Erlang/OTP ${{matrix.otp}})
+    runs-on: windows-2019
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+          - elixir: 1.12.x
+            otp: 22.x
+          - elixir: 1.12.x
+            otp: 23.x
+          - elixir: 1.13.x
+            otp: 22.x
+          - elixir: 1.13.x
+            otp: 23.x
+          - elixir: 1.13.x
+            otp: 24.x
+          - elixir: 1.13.x
+            otp: 25.x
+          - elixir: 1.14.x
+            otp: 23.x
+          - elixir: 1.14.x
+            otp: 24.x
+          - elixir: 1.14.x
+            otp: 25.x
+    env:
+      MIX_ENV: test
+    steps:
+      - name: Set git to use original line ending
+        run: |
+          git config --global core.autocrlf false
+      - uses: actions/checkout@v3
+      - uses: erlef/setup-beam@v1
+        with:
+          otp-version: ${{matrix.otp}}
+          elixir-version: ${{matrix.elixir}}
+      - name: Install Dependencies
+        run: |
+          mix local.hex --force
+          mix local.rebar --force
+          mix deps.get --only test
+      - run: mix test
 
   static_analysis:
     name: static analysis (Elixir ${{matrix.elixir}} | Erlang/OTP ${{matrix.otp}})

apps/language_server/lib/language_server/experimental/code_mod/format.ex

@@ -3,6 +3,7 @@ defmodule ElixirLS.LanguageServer.Experimental.CodeMod.Format do
   alias ElixirLS.LanguageServer.Experimental.SourceFile
   alias ElixirLS.LanguageServer.Experimental.SourceFile.Conversions
   alias ElixirLS.LanguageServer.Experimental.Protocol.Types.TextEdit
+  alias ElixirLS.LanguageServer.SourceFile.Path, as: SourceFilePath
 
   require Logger
   @type formatter_function :: (String.t() -> any) | nil
@@ -102,8 +103,8 @@ defmodule ElixirLS.LanguageServer.Experimental.CodeMod.Format do
       :ok
     else
       message =
-        "Cannot format file from current directory " <>
-          "(Currently in #{Path.relative_to(cwd, project_path)})"
+        "Cannot format '#{document.path}' from current directory " <>
+          "(Currently in #{project_path})"
 
       {:error, message}
     end
@@ -112,31 +113,35 @@ defmodule ElixirLS.LanguageServer.Experimental.CodeMod.Format do
   defp check_inputs_apply(%SourceFile{} = document, project_path, inputs)
        when is_list(inputs) do
     formatter_dir = dominating_formatter_exs_dir(document, project_path)
+    # document.path is native, convert to universal separators
+    document_path = Path.absname(document.path)
 
     inputs_apply? =
       Enum.any?(inputs, fn input_glob ->
         glob =
           if Path.type(input_glob) == :relative do
-            Path.join(formatter_dir, input_glob)
+            formatter_dir
+            |> Path.join(input_glob)
           else
             input_glob
           end
 
-        PathGlobVendored.match?(document.path, glob, match_dot: true)
+        PathGlobVendored.match?(document_path, glob, match_dot: true)
       end)
 
     if inputs_apply? do
       :ok
     else
-      {:error, :input_mismatch}
+      {:error, {:input_mismatch, "#{document_path} is not matched by #{inspect(inputs)}"}}
     end
   end
 
   defp check_inputs_apply(_, _, _), do: :ok
 
   defp subdirectory?(child, parent: parent) do
     normalized_parent = Path.absname(parent)
-    String.starts_with?(child, normalized_parent)
+    normalized_child = Path.absname(child)
+    String.starts_with?(normalized_child, normalized_parent)
   end
 
   # Finds the directory with the .formatter.exs that's the nearest parent to the

apps/language_server/lib/language_server/source_file/path.ex

@@ -41,6 +41,10 @@ defmodule ElixirLS.LanguageServer.SourceFile.Path do
     uri |> from_uri |> Path.absname()
   end
 
+  def absolute(path) do
+    path |> Path.expand() |> convert_separators_to_native()
+  end
+
   def to_uri(path) do
     path =
       path
@@ -83,6 +87,13 @@ defmodule ElixirLS.LanguageServer.SourceFile.Path do
     |> URI.to_string()
   end
 
+  def windows? do
+    case os_type() do
+      {:win32, _} -> true
+      _ -> false
+    end
+  end
+
   defp convert_separators_to_native(path) do
     if windows?() do
       # convert path separators from URI to Windows
@@ -101,13 +112,6 @@ defmodule ElixirLS.LanguageServer.SourceFile.Path do
     end
   end
 
-  defp windows? do
-    case os_type() do
-      {:win32, _} -> true
-      _ -> false
-    end
-  end
-
   # this is here to be mocked in tests
   defp os_type do
     :os.type()

apps/language_server/test/experimental/code_mod/format_test.exs

@@ -1,6 +1,7 @@
 defmodule ElixirLS.Experimental.FormatTest do
   alias ElixirLS.LanguageServer.Experimental.CodeMod.Format
   alias ElixirLS.LanguageServer.Experimental.SourceFile
+  alias ElixirLS.LanguageServer.SourceFile.Path, as: SourceFilePath
 
   use ElixirLS.Test.CodeMod.Case
 
@@ -13,7 +14,9 @@ defmodule ElixirLS.Experimental.FormatTest do
   end
 
   def source_file(text) do
-    SourceFile.new("file://#{__ENV__.file}", text, 1)
+    __ENV__.file
+    |> SourceFilePath.to_uri()
+    |> SourceFile.new(text, 1)
   end
 
   def unformatted do

apps/language_server/test/experimental/project_test.exs

@@ -1,6 +1,7 @@
 defmodule ElixirLS.Experimental.ProjectTest do
   alias ElixirLS.LanguageServer.Experimental.Project
   alias ElixirLS.LanguageServer.SourceFile
+  alias ElixirLS.LanguageServer.Test.Paths
 
   use ExUnit.Case, async: false
   use Patch
@@ -30,11 +31,13 @@ defmodule ElixirLS.Experimental.ProjectTest do
   end
 
   def root_uri do
-    "file:///tmp/my_project"
+    System.tmp_dir!()
+    |> Path.join("my_project")
+    |> SourceFile.Path.to_uri()
   end
 
   def with_a_root_uri(_) do
-    {:ok, root_uri: "file:///tmp/my_project"}
+    {:ok, root_uri: root_uri()}
   end
 
   setup do
@@ -240,35 +243,40 @@ defmodule ElixirLS.Experimental.ProjectTest do
         String.ends_with?(path, "mix.exs")
       end)
 
-      assert {:ok, %Project{} = project} =
-               Project.change_project_directory(ctx.project, "sub_dir/new/dir")
+      sub_dir = Path.join(~w(sub_dir new dir))
+      assert {:ok, %Project{} = project} = Project.change_project_directory(ctx.project, sub_dir)
+
+      assert Project.project_path(project) ==
+               [File.cwd!(), "sub_dir", "new", "dir"]
+               |> Path.join()
+               |> Paths.to_native_separators()
 
-      assert Project.project_path(project) == "#{File.cwd!()}/sub_dir/new/dir"
       assert project.mix_project?
     end
 
     test "only sets the project directory if the root uri is set" do
       project = Project.new(nil)
+      sub_dir = Path.join(~w(sub_dir new dir))
 
-      assert {:ok, project} = Project.change_project_directory(project, "sub_dir/new/dir")
+      assert {:ok, project} = Project.change_project_directory(project, sub_dir)
       assert project.root_uri == nil
       assert Project.project_path(project) == nil
     end
 
     test "defaults to the root uri's directory", ctx do
       assert {:ok, project} = Project.change_project_directory(ctx.project, nil)
       root_path = SourceFile.Path.absolute_from_uri(project.root_uri)
-      assert Project.project_path(project) == root_path
+      assert Project.project_path(project) == Paths.to_native_separators(root_path)
     end
 
     test "defaults to the root uri's directory if the project directory is empty", ctx do
       assert {:ok, project} = Project.change_project_directory(ctx.project, "")
       root_path = SourceFile.Path.absolute_from_uri(project.root_uri)
-      assert Project.project_path(project) == root_path
+      assert Project.project_path(project) == Paths.to_native_separators(root_path)
     end
 
     test "normalizes the project directory", ctx do
-      subdirectory = "sub_dir/../sub_dir/new/../new/dir"
+      subdirectory = Path.join(~w(sub_dir .. sub_dir new .. new dir))
 
       patch(File, :exists?, fn path, _ ->
         String.ends_with?(path, "mix.exs")
@@ -277,29 +285,41 @@ defmodule ElixirLS.Experimental.ProjectTest do
       assert {:ok, %Project{} = project} =
                Project.change_project_directory(ctx.project, subdirectory)
 
-      assert Project.project_path(project) == "#{File.cwd!()}/sub_dir/new/dir"
+      assert sub_dir = Path.join([File.cwd!(), "sub_dir", "new", "dir"])
+      assert Project.project_path(project) == Paths.to_native_separators(sub_dir)
       assert project.mix_project?
-      assert Project.mix_exs_path(project) == "#{File.cwd!()}/sub_dir/new/dir/mix.exs"
+
+      assert Project.mix_exs_path(project) ==
+               sub_dir
+               |> Path.join("mix.exs")
+               |> Paths.to_native_separators()
     end
 
     test "sets mix project to false if the mix.exs doesn't exist", ctx do
       patch(File, :exists?, fn file_name ->
         !String.ends_with?(file_name, "mix.exs")
       end)
 
-      assert {:ok, %Project{} = project} =
-               Project.change_project_directory(ctx.project, "sub_dir/new/dir")
+      sub_dir = Path.join(~w(sub_dir new dir))
+      assert {:ok, %Project{} = project} = Project.change_project_directory(ctx.project, sub_dir)
+
+      assert Project.project_path(project) ==
+               File.cwd!()
+               |> Path.join(sub_dir)
+               |> Paths.to_native_separators()
 
-      assert Project.project_path(project) == "#{File.cwd!()}/sub_dir/new/dir"
       refute project.mix_project?
     end
 
     test "asks for a restart if the project directory was set and the new one isn't the same",
          ctx do
-      {:ok, project} = Project.change_project_directory(ctx.project, "sub_dir/foo")
+      foo_sub_dir = Path.join(~w(sub_dir foo))
+      {:ok, project} = Project.change_project_directory(ctx.project, foo_sub_dir)
+
+      new_dir_sub_dir = Path.join(~w(sub_dir new dir))
 
       assert {:restart, :warning, message} =
-               Project.change_project_directory(project, "sub_dir/new/dir")
+               Project.change_project_directory(project, new_dir_sub_dir)
 
       assert message =~ "Project directory change detected"
     end
@@ -308,7 +328,8 @@ defmodule ElixirLS.Experimental.ProjectTest do
       {:ok, project} = Project.change_project_directory(ctx.project, "sub_dir/foo")
 
       patch(File, :dir?, false)
-      new_directory = "sub_dir/new/dir"
+
+      new_directory = Path.join(~w(sub_dir new dir))
       expected_message_directory = Path.join(File.cwd!(), new_directory)
 
       assert {:error, message} = Project.change_project_directory(project, new_directory)
@@ -317,8 +338,9 @@ defmodule ElixirLS.Experimental.ProjectTest do
 
     test "rejects a change if the project directory isn't a subdirectory of the project root",
          ctx do
-      assert {:error, message} =
-               Project.change_project_directory(ctx.project, "../../../../not-a-subdir")
+      not_in_project = Path.join(~w(.. .. .. .. not-a-subdir))
+
+      assert {:error, message} = Project.change_project_directory(ctx.project, not_in_project)
 
       assert message =~ "is not a subdirectory of"
     end