fix: reduce locking (#14)

* fix: remove semaphores

* error handling

* fix

* improve error handling

* support disabling MTP with MSBuild property

* improve caching

* fix table equality bug

* improve test discovery

* fix tests

* add backup client caching

* add __eq to project info

* isolate tests

* bump test timeout

* fix running empty file

Author: Nsidorenco

lua/neotest-vstest/client.lua

@@ -6,104 +6,70 @@ local dotnet_utils = require("neotest-vstest.dotnet_utils")
 
 local client_discovery = {}
 
-local client_creation_semaphore = nio.control.semaphore(1)
 local clients = {}
 
 ---@param project DotnetProjectInfo?
 ---@param solution string? path to the solution file
 ---@return neotest-vstest.Client?
 function client_discovery.get_client_for_project(project, solution)
   if not project then
+    logger.debug("neotest-vstest: No project provided, returning nil client.")
     return nil
   end
 
-  ---@type neotest-vstest.Client | boolean
-  local client = false
+  if clients[project.proj_file] ~= nil then
+    return clients[project.proj_file] or nil
+  end
 
-  client_creation_semaphore.with(function()
-    if clients[project.proj_file] ~= nil then
-      client = clients[project.proj_file]
-      return
-    end
+  -- Check if the project is part of a solution.
+  -- If not then do not create a client.
+  local solution_projects = solution and dotnet_utils.get_solution_projects(solution)
+  if solution_projects and #solution_projects.projects > 0 then
+    local project_files = vim.tbl_map(function(proj)
+      return proj.proj_file
+    end, solution_projects.projects)
 
-    -- Check if the project is part of a solution.
-    -- If not then do not create a client.
-    local solution_projects = solution and dotnet_utils.get_solution_projects(solution)
-    if solution_projects and #solution_projects.projects > 0 then
-      if not vim.list_contains(solution_projects.projects, project) then
-        logger.debug(
-          "neotest-vstest: project is not part of the solution projects: "
-            .. vim.inspect(solution_projects.projects)
-            .. ", project: "
-            .. vim.inspect(project)
-        )
-        clients[project.proj_file] = client
-        return
-      end
+    if not vim.list_contains(project_files, project.proj_file) then
       logger.debug(
-        "neotest-vstest: project is part of the solution projects: " .. vim.inspect(project)
+        "neotest-vstest: project is not part of the solution projects: "
+          .. vim.inspect(solution_projects.projects)
+          .. ", project: "
+          .. vim.inspect(project)
       )
-    else
-      logger.debug(
-        "neotest-vstest: no solution projects found, using solution: " .. vim.inspect(solution)
-      )
-    end
-
-    -- Project is part of a solution or standalone, create a client.
-    if project.is_mtp_project then
-      logger.debug(
-        "neotest-vstest: Creating mtp client for project "
-          .. project.proj_file
-          .. " and "
-          .. project.dll_file
-      )
-      client = mtp_client:new(project)
-    elseif project.is_test_project then
-      client = vstest_client:new(project)
+      clients[project.proj_file] = false
+      return
     end
-    clients[project.proj_file] = client
-  end)
-
-  if client == false then
-    return nil
   else
-    return client
-  end
-end
-
-local solution_cache
-local solution_semaphore = nio.control.semaphore(1)
-
-function client_discovery.discover_solution_tests(root)
-  if solution_cache then
-    return solution_cache
+    logger.debug(
+      "neotest-vstest: no solution projects found for solution: " .. vim.inspect(solution)
+    )
   end
 
-  solution_semaphore.acquire()
-
-  local res = dotnet_utils.get_solution_projects(root)
-
-  dotnet_utils.build_path(root)
-
-  local project_clients = {}
-
-  for _, project in ipairs(res.projects) do
-    if project.is_test_project or project.is_mtp_project then
-      project_clients[project.proj_file] = client_discovery.get_client_for_project(project)
-    end
-  end
-
-  logger.debug("neotest-vstest: discovered projects:")
-  logger.debug(res.projects)
-
-  for _, client in ipairs(project_clients) do
-    local project_tests = client:discover_tests()
-    vim.tbl_extend("force", solution_cache, project_tests)
+  ---@type neotest-vstest.Client
+  local client
+
+  -- Project is part of a solution or standalone, create a client.
+  if project.is_mtp_project then
+    logger.debug(
+      "neotest-vstest: Creating mtp client for project "
+        .. project.proj_file
+        .. " and "
+        .. project.dll_file
+    )
+    client = mtp_client:new(project)
+  elseif project.is_test_project then
+    client = vstest_client:new(project)
+  else
+    logger.warn(
+      "neotest-vstest: Project is neither test project nor mtp project, returning nil client for "
+        .. project.proj_file
+    )
+    clients[project.proj_file] = false
+    return
   end
 
-  solution_semaphore.release()
-
-  return solution_cache
+  clients[project.proj_file] = client
+  return client
 end
 
 return client_discovery

lua/neotest-vstest/dotnet_utils.lua

@@ -184,6 +184,7 @@ function dotnet_utils.get_proj_info(path)
     "-getProperty:MSBuildProjectDirectory",
     "-getProperty:IsTestProject",
     "-getProperty:IsTestingPlatformApplication",
+    "-getProperty:DisableTestingPlatformServerCapability",
     "-property:TargetFramework=" .. target_framework,
   }
 
@@ -198,14 +199,23 @@ function dotnet_utils.get_proj_info(path)
   logger.debug("neotest-vstest: msbuild properties for " .. proj_file .. ":")
   logger.debug(properties)
 
+  local is_mtp_disabled = properties.DisableTestingPlatformServerCapability == "true"
+
+  ---@class DotnetProjectInfo
   local proj_data = {
     proj_file = proj_file,
     dll_file = properties.TargetPath,
     proj_dir = properties.MSBuildProjectDirectory,
     is_test_project = properties.IsTestProject == "true",
-    is_mtp_project = properties.IsTestingPlatformApplication == "true",
+    is_mtp_project = not is_mtp_disabled and properties.IsTestingPlatformApplication == "true",
   }
 
+  setmetatable(proj_data, {
+    __eq = function(a, b)
+      return a.proj_file == b.proj_file
+    end,
+  })
+
   if proj_data.dll_file == "" then
     logger.debug("neotest-vstest: failed to find dll file for " .. proj_file)
     logger.debug(path)
@@ -313,6 +323,7 @@ function dotnet_utils.build_path(path)
     nio.scheduler()
     logger.error("neotest-vstest: failed to build path " .. path)
     logger.error(out.stdout)
+    logger.error(out.stderr)
     vim.notify_once("neotest-vstest: failed to build project " .. path, vim.log.levels.ERROR)
     return false
   end

lua/neotest-vstest/init.lua

@@ -69,7 +69,6 @@ function DotnetNeotestAdapter.root(path)
 
     if solution_dir_future.wait() then
       logger.info(string.format("neotest-vstest: found solution file %s", solution))
-      client_discovery.discover_solution_tests(solution)
       return solution_dir
     end
   end
@@ -79,24 +78,24 @@ function DotnetNeotestAdapter.root(path)
 end
 
 function DotnetNeotestAdapter.is_test_file(file_path)
-  local isTestFile = (vim.endswith(file_path, ".csproj") or vim.endswith(file_path, ".fsproj"))
+  local isDotnetFile = (vim.endswith(file_path, ".csproj") or vim.endswith(file_path, ".fsproj"))
     or (vim.endswith(file_path, ".cs") or vim.endswith(file_path, ".fs"))
 
-  if not isTestFile then
+  if not isDotnetFile then
     return false
   end
 
   local project = dotnet_utils.get_proj_info(file_path)
   local client = client_discovery.get_client_for_project(project, solution)
 
-  local tests = (client and client:discover_tests_for_path(file_path)) or {}
-
-  local n = 0
-  if tests then
-    n = #vim.tbl_values(tests)
+  if not client then
+    logger.debug(
+      "neotest-vstest: marking file as non-test file since no client was found: " .. file_path
+    )
+    return false
   end
 
-  return tests and n > 0
+  return true
 end
 
 function DotnetNeotestAdapter.filter_dir(name)
@@ -183,7 +182,6 @@ local function build_position(source, captured_nodes, tests_in_file, path)
             type = match_type,
             path = path,
             name = test.DisplayName,
-            qualified_name = test.FullyQualifiedName,
             range = { definition:range() },
           })
           tests_in_file[id] = nil
@@ -224,15 +222,22 @@ local function get_top_level_tests(project)
 
   local client = client_discovery.get_client_for_project(project, solution)
 
-  local tests_in_file = (client and client:discover_tests()) or {}
+  if not client then
+    logger.debug(
+      "neotest-vstest: not discovering top-level tests due to no client for project: "
+        .. vim.inspect(project)
+    )
+  end
 
+  local tests_in_file = (client and client:discover_tests()) or {}
+  local tests_in_project = tests_in_file[project.proj_file]
   logger.debug(string.format("neotest-vstest: top-level tests in file: %s", project.dll_file))
 
-  if not tests_in_file or next(tests_in_file) == nil then
+  if not tests_in_project or next(tests_in_project) == nil then
     return
   end
 
-  local n = #tests_in_file
+  local n = #tests_in_project
 
   local nodes = {
     {
@@ -246,18 +251,21 @@ local function get_top_level_tests(project)
   local i = 0
 
   -- add tests which does not have a matching tree-sitter node.
-  for id, test in pairs(tests_in_file) do
+  for id, test in pairs(tests_in_project) do
     nodes[#nodes + 1] = {
       id = id,
       type = "test",
-      path = project.proj_file,
+      path = test.CodeFilePath,
       name = test.DisplayName,
-      qualified_name = test.FullyQualifiedName,
       range = { i, 0, i + 1, -1 },
     }
     i = i + 1
   end
 
+  if #nodes <= 1 then
+    return {}
+  end
+
   local structure = assert(build_structure(nodes, {}, {
     nested_tests = false,
     require_namespaces = false,
@@ -288,6 +296,9 @@ function DotnetNeotestAdapter.discover_positions(path)
   local client = client_discovery.get_client_for_project(project, solution)
 
   if not client then
+    logger.debug(
+      "neotest-vstest: not discovering tests due to no client for file: " .. vim.inspect(path)
+    )
     return
   end
 
@@ -345,20 +356,24 @@ function DotnetNeotestAdapter.discover_positions(path)
 
     -- add tests which does not have a matching tree-sitter node.
     for id, test in pairs(tests_in_file) do
+      local line = test.LineNumber or 0
       nodes[#nodes + 1] = {
         id = id,
         type = "test",
         path = path,
         name = test.DisplayName,
-        qualified_name = test.FullyQualifiedName,
-        range = { test.LineNumber - 1, 0, test.LineNumber - 1, -1 },
+        range = { line - 1, 0, line - 1, -1 },
       }
     end
 
     for _, node in ipairs(nodes) do
       node.client = client
     end
 
+    if #nodes <= 1 then
+      return {}
+    end
+
     local structure = assert(build_structure(nodes, {}, {
       nested_tests = false,
       require_namespaces = false,
@@ -430,7 +445,7 @@ function DotnetNeotestAdapter.build_spec(args)
   }
 end
 
-function DotnetNeotestAdapter.results(spec, result, _tree)
+function DotnetNeotestAdapter.results(spec, result)
   logger.info("neotest-vstest: waiting for test results")
   logger.debug(spec)
   logger.debug(result)
@@ -440,7 +455,7 @@ function DotnetNeotestAdapter.results(spec, result, _tree)
   if not results then
     for _, id in ipairs(vim.tbl_values(spec.context.projects_id_map)) do
       results[id] = {
-        status = types.ResultStatus.failed,
+        status = types.ResultStatus.skipped,
         output = spec.context.result_path,
         errors = {
           { message = result.output },