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

chore: POC plenary test #3063

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

chore: POC plenary test #3063

wants to merge 6 commits into from

Conversation

alex-courtis
Copy link
Member

Plenary test is used by
telescope
lualine
packer

It allows static mocking of nvim functions.

@gegoune
Copy link
Collaborator

gegoune commented Feb 1, 2025

I can't recall where and when it was but I have seen projects moving away from plenary tests. Might be worth exploring what top plugins in the ecosystem use/do.

Copy link
Collaborator

@GCrispino GCrispino left a comment

Choose a reason for hiding this comment

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

I don't have a lot of experience in the neovim development space, so I don't think I can weigh in too much here, sorry :/ .

I guess my only question would be setup wise. I checked the branch out on MacOS and ran the script in ./scripts/test.sh, but got an error - I believe I need to install plenary or something like that?

Comment on lines 8 to 9
before_each(function()
end)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this intended?

Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops... didn't need that one...

scripts/test.sh Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we add a command in Makefile (e.g. make test) that runs this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup. It can then run in CI.

@GCrispino
Copy link
Collaborator

I can't recall where and when it was but I have seen projects moving away from plenary tests. Might be worth exploring what top plugins in the ecosystem use/do.

I know very little about this but I watched this video on the subject, which uses plenary and mentions mini.test.

@alex-courtis
Copy link
Member Author

I guess my only question would be setup wise. I checked the branch out on MacOS and ran the script in ./scripts/test.sh, but got an error - I believe I need to install plenary or something like that?

Whoops... forgot to add that step - check out plenary at ./plenary.nvim

Early days; many thanks for taking a look!

@alex-courtis
Copy link
Member Author

I know very little about this but I watched this video on the subject, which uses plenary and mentions mini.test.

The mini plugins are rather good, looks promising. I have homework to do.

@alex-courtis alex-courtis reopened this Feb 2, 2025
@alex-courtis
Copy link
Member Author

Tidied and added missing bits.

It looks like plenary is a subset of busted, which isn't well liked any more.

Let's see how mini goes...

@alex-courtis
Copy link
Member Author

I gave mini.test a go:

  • optional busted (plenary) compatibility
  • nice style of laying out tests via sets
  • high level of control over child vim process
  • all the mini plugins have very comprehensive tests

Thoughts:

  • sets are nice, but way too many strings; could probably use table keys directly
  • asserts are pretty basic
  • no plugins but mini uses it

Dealbreaker:

My Conclusion:

  • Plenary works nicely

Same utils test in mini; didn't setup the harnesses, which are very close to plenary anyway.

local utils = require("nvim-tree.utils")

local new_set = MiniTest.new_set
local eq = MiniTest.expect.equality

-- Create (but not start) child Neovim object
local child = MiniTest.new_child_neovim()

local T = new_set({
  hooks = {
    post_once = child.stop,
  },
})

T["utils.path_add_trailing()"] = new_set()

T["utils.path_add_trailing()"]["trailing added"] = function()
  eq("foo/", utils.path_add_trailing("foo"))
end

T["utils.path_add_trailing()"]["trailing already present"] = function()
  eq("foo/", utils.path_add_trailing("foo/"))
end

T["utils.canonical_path()"] = new_set()

T["utils.canonical_path()"]["not windows"] = function()
  eq("c:\\foo\\bar", utils.canonical_path("c:\\foo\\bar"))
end

T["utils.canonical_path()"]["is windows"] = function()
  -- missing the ability to mock/stub
  -- https://github.com/echasnovski/mini.test/blob/main/doc/mini-test.txt#L37

  -- vim.fn.has.on_call_with("win32unix").returns(1)
  eq("c:\\foo\\bar", utils.canonical_path("C:\\foo\\bar"))
end

return T

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.

3 participants