Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update test_harness.lua #476

Merged
merged 4 commits into from
May 24, 2023
Merged

Update test_harness.lua #476

merged 4 commits into from
May 24, 2023

Conversation

derekthecool
Copy link
Contributor

Add support for test harness on windows. Using the find command does not work on windows, these changes should fix the problem.

@derekthecool
Copy link
Contributor Author

It is getting further than before, but I'm still getting stuck and not able to run tests. I've been snooping around but have not been able to find any other reasons that might be blocking on windows.

My output that I'm getting with two test files available looks like this

nvim --headless -c "PlenaryBustedDirectory ."
Starting...Scheduling: tests\bit-flip_spec.lua
Scheduling: tests\comma_count_spec.lua

It freezes here for about 20 seconds then exits without any more output. I've also tried using vim.fs.normalize on the file paths but that didn't fix it either.

This is the normalize testing I've done

function harness._find_files_to_run(directory)
  -- Default use the find command
  local find_command = "find"
  local find_args = { "-type", "f", "-name", "*_spec.lua" }

  -- On windows use powershell Get-ChildItem instead
  if vim.fn.has('win32') == 1 or vim.fn.has('win64') == 1 then
    find_command = "powershell"
    find_args = { '-Command', [[Get-ChildItem -Recurse -n -Filter "*_spec.lua"]] }
  end

  local finder = Job:new {
    command = find_command,
    args = find_args,
    cwd = directory,
  }

  local files = finder:sync()
  for k,v in pairs(files) do
      files[k] = './' .. vim.fs.normalize(v)
  end

  local output = vim.tbl_map(Path.new, files)
  return output
end

Any ideas of additional things to try here?

@derekthecool
Copy link
Contributor Author

Good news, I've found the last spot causing issues running the test harness on windows.

I tried using the sequential testing and that gave me more meaningful errors than running parallel tests.
The error message said something like Invalid escape sequence near D:. I am using a D:\ drive so I knew right away that there was a backslash escape error somewhere.

Luckily after some digging I found the location.
This code blob in test_harness.lua

    local args = {
      "--headless",
      "-c",
      string.format('lua require("plenary.busted").run("%s")', p:absolute()),
    }

Just needed this small gsub change

    local args = {
      "--headless",
      "-c",
      string.format('lua require("plenary.busted").run("%s")', p:absolute():gsub("\\","\\\\")),
    }

I looked into making a change at the source of the absolute function. But I got really confused by all the metatable magic and was worried I'd make more problems. If you'd prefer, I can adjust these changes. I just wanted something that worked.

I'm also a slight bit worried that this could mess up other paths that have a backslash for something other than Windows path separators.

I can confirm now that it does fully work on Windows. I've also tested again on Linux and proved I didn't break anything.

To summarize my two commits:

  • Add method to find test spec files on windows using Get-ChildItem instead of find which is unavailable
  • Add needed extra escape for Windows path separators

derekthecool and others added 2 commits May 24, 2023 08:55
Add support for test harness on windows. Using the find command does not work on windows, these changes should fix the problem.
@Conni2461
Copy link
Collaborator

thats awesome, i try to figure out why linux is borked and then merge. thanks you very much :)

@Conni2461
Copy link
Collaborator

i hope i didn't broke windows in the process ^^

@Conni2461 Conni2461 merged commit f7db41f into nvim-lua:master May 24, 2023
5 checks passed
@derekthecool
Copy link
Contributor Author

Awesome, yay! Yep everything is running fine for me still.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants