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

feat(tests): adds test management #642

Merged
merged 3 commits into from May 22, 2023
Merged

feat(tests): adds test management #642

merged 3 commits into from May 22, 2023

Conversation

Jomik
Copy link
Contributor

@Jomik Jomik commented Apr 20, 2023

Adds neotest setup based on my very early understanding of neotest.

We can then add test adapters in the following way.

return {
  {
    "nvim-neotest/neotest",
    dependencies = {
      "nvim-neotest/neotest-go",
    },
    opts = function(_, opts)
      -- Would like "cleaner" way than this..
      opts.adapters = vim.list_extend(opts.adapters or {}, {
        require("neotest-go")({ args = { "-tags=integration" } }),
      })
    end,
  },
}

Can we somehow only add the "Debug Nearest" keybind if nvim-dap exists?

@loqusion
Copy link
Contributor

loqusion commented Apr 27, 2023

antoinemadec/FixCursorHold.nvim may need to be added as a dependency even though part of its functionality has already been merged into neovim:

Neotest uses the CursorHold event. This uses the updatetime setting which is by default very high, and lowering this can lead to excessive writes to disk. It's recommended to use https://github.com/antoinemadec/FixCursorHold.nvim which allows detaching updatetime from the frequency of the CursorHold event. The repo claims it is no longer needed but it is still recommended (See this issue)

@Jomik
Copy link
Contributor Author

Jomik commented Apr 28, 2023

antoinemadec/FixCursorHold.nvim may need to be added as a dependency even though part of its functionality has already been merged into neovim:

I am unsure of the impact - decided to not add it as I do not understand it fully.
It should be easy to add from userland. But I am definitely not against adding it either.
Just need to be told what is "correct" 😄

@Jomik
Copy link
Contributor Author

Jomik commented Apr 29, 2023

@folke What do you think about this? Do you prefer me opening an Idea in discussions?

@kevinrobayna
Copy link
Contributor

kevinrobayna commented May 2, 2023

I would love to have this tbh! Something else i would add to the default configuration is neotest as it's a best effort plugin to run test automatically. see my config for neotest https://github.com/kevinrobayna/dotfiles/blob/c699b32f7493e0caf44dc94db738447fd41b03f9/config/.config/nvim/lua/plugins/neotest.lua

require("neotest-vim-test")({
  ignore_file_types = { "ruby", "go" },
})

Although i think is also makes sense to leave it to the consumer so that they do something like this

  {
    "nvim-neotest/neotest",
    dependencies = {
      "nvim-neotest/neotest-vim-test",
    },
    opts = function(_, opts)
      opts.adapters = vim.list_extend(opts.adapters or {}, {
        require("neotest-vim-test")({
          ignore_file_types = { "ruby", "go" },
        }),
      })
    end,
  },

@Jomik
Copy link
Contributor Author

Jomik commented May 4, 2023

I would love to have this tbh! Something else i would add to the default configuration is neotest as it's a best effort plugin to run test automatically. see my config for neotest https://github.com/kevinrobayna/dotfiles/blob/c699b32f7493e0caf44dc94db738447fd41b03f9/config/.config/nvim/lua/plugins/neotest.lua

require("neotest-vim-test")({
  ignore_file_types = { "ruby", "go" },
})

Although i think is also makes sense to leave it to the consumer so that they do something like this

  {
    "nvim-neotest/neotest",
    dependencies = {
      "nvim-neotest/neotest-vim-test",
    },
    opts = function(_, opts)
      opts.adapters = vim.list_extend(opts.adapters or {}, {
        require("neotest-vim-test")({
          ignore_file_types = { "ruby", "go" },
        }),
      })
    end,
  },

I would personally prefer not to have neotest-vim-test as a dependency, it seems like that is only there because there is not a specific adapter for all languages yet, so this work as backwards compatible bridge to vim-test.

I am not sure how this relates to auto running tests - unless vim-test does that?
But if I understand correctly, you can auto run tests using neotest with an auto command.
Not sure if we want to include that in a default configuration either though?

@folke
Copy link
Collaborator

folke commented May 4, 2023

Sorry for the delay. Will have a look into this tomorrow!

@kevinrobayna
Copy link
Contributor

I would love to have this tbh! Something else i would add to the default configuration is neotest as it's a best effort plugin to run test automatically. see my config for neotest https://github.com/kevinrobayna/dotfiles/blob/c699b32f7493e0caf44dc94db738447fd41b03f9/config/.config/nvim/lua/plugins/neotest.lua

require("neotest-vim-test")({

ignore_file_types = { "ruby", "go" },

})

Although i think is also makes sense to leave it to the consumer so that they do something like this

{

"nvim-neotest/neotest",
dependencies = {
  "nvim-neotest/neotest-vim-test",
},
opts = function(_, opts)
  opts.adapters = vim.list_extend(opts.adapters or {}, {
    require("neotest-vim-test")({
      ignore_file_types = { "ruby", "go" },
    }),
  })
end,

},

I would personally prefer not to have neotest-vim-test as a dependency, it seems like that is only there because there is not a specific adapter for all languages yet, so this work as backwards compatible bridge to vim-test.

I am not sure how this relates to auto running tests - unless vim-test does that?

But if I understand correctly, you can auto run tests using neotest with an auto command.

Not sure if we want to include that in a default configuration either though?

Yeah I think the default configuration should be as minimal as possible to be honest

@ditsuke
Copy link
Contributor

ditsuke commented May 6, 2023

I recently expedited some effort in setting up neotest with language plugins broken up into language layers:

  1. Core config with ships with neotest-vim-test: https://github.com/ditsuke/nvim-config/blob/main/lua/ditsuke/extras/code/neotest.lua
  2. Language layer adds neotest adapter: https://github.com/ditsuke/nvim-config/blob/main/lua/ditsuke/extras/lang/go.lua#L40-L41

Since the language extras in my configuration are dependent on the Neotest extra, I had to use a hack to install neotest plugins only if it is already enabled. I believe this is an issue we want resolved for Lazyvim's official neotest layer too so its something I'd like us to consider a potentially better solution for.

@Jomik
Copy link
Contributor Author

Jomik commented May 7, 2023

I recently expedited some effort in setting up neotest with language plugins broken up into language layers:

  1. Core config with ships with neotest-vim-test: https://github.com/ditsuke/nvim-config/blob/main/lua/ditsuke/extras/code/neotest.lua

Why do you want neotest-vim-test? It seems like a "catch-all" / "bloated" package.
We want to stay minimal, so we should really only have language specific adapters. neotest-vim-test seems easy to add in userland anyway?

@ditsuke
Copy link
Contributor

ditsuke commented May 7, 2023

I recently expedited some effort in setting up neotest with language plugins broken up into language layers:

  1. Core config with ships with neotest-vim-test: https://github.com/ditsuke/nvim-config/blob/main/lua/ditsuke/extras/code/neotest.lua

Why do you want neotest-vim-test? It seems like a "catch-all" / "bloated" package.
We want to stay minimal, so we should really only have language specific adapters. neotest-vim-test seems easy to add in userland anyway?

I'm fine either way wrt vim-test. My concern is having a clean way to enable neotest adaptors for imported language layers. Although one clear advantage to vim-test is having neotest ready to go for any language/framework OOTB, some for which native adaptors don't exist at all.

@Jomik
Copy link
Contributor Author

Jomik commented May 22, 2023

@folke Did you have time to give feedback here? 😄

Copy link
Collaborator

@folke folke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

lua/lazyvim/plugins/extras/test/core.lua Outdated Show resolved Hide resolved
lua/lazyvim/plugins/extras/test/core.lua Outdated Show resolved Hide resolved
@folke
Copy link
Collaborator

folke commented May 22, 2023

Let me know when it's ready to merge

@Jomik
Copy link
Contributor Author

Jomik commented May 22, 2023

Let me know when it's ready to merge

Just copied into my personal config and it works as expected.
Unsure if there is a good way to test other than that.. 😄
Do we need any documentation / examples on how to configure a test adapter/plugin?
I only have neotest-go as a working example currently.

  {
    "nvim-neotest/neotest",
    dependencies = {
      "nvim-neotest/neotest-go",
    },
    opts = function(_, opts)
      opts.adapters = vim.list_extend(opts.adapters or {}, {
        require("neotest-go")({
          args = { "-tags=integration" },
        }),
      })
    end,
  },

@folke
Copy link
Collaborator

folke commented May 22, 2023

Looks good. I don't really use neotest myself, but may give it a try :)

@folke folke merged commit bb0d4d4 into LazyVim:main May 22, 2023
3 checks passed
@folke
Copy link
Collaborator

folke commented May 22, 2023

Merged. Thanks!

@Jomik Jomik deleted the extras.test branch May 22, 2023 18:58
ofrades pushed a commit to ofrades/LazyVim that referenced this pull request May 30, 2023
* feat(tests): adds test management

* refactor(tests): pr changes

* fix(tests): make dap keybind optional based on nvim-dap existence
joshmedeski pushed a commit to joshmedeski/LazyVim that referenced this pull request Sep 1, 2023
* feat(tests): adds test management

* refactor(tests): pr changes

* fix(tests): make dap keybind optional based on nvim-dap existence
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

5 participants