-
Notifications
You must be signed in to change notification settings - Fork 34
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
tests(ui): add unit tests for UI functionality #240
Conversation
@@ -83,14 +83,17 @@ local open_buffer = function() | |||
vim.api.nvim_set_current_win(prev_win) | |||
end | |||
|
|||
local close_buffer = function() | |||
M._close_buffer = function() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I spent some time experimenting with a various approaches towards the testing of UI. I think that going full functional with a msgpack and remote headless instance is not necessary and the testing can stay on unit level with stubs. There's no need to test things like curl, jq or http servers either :-).
However as internal helpers like close_buffer can't be mocked and call two vim functions already. The development experience of mocking the second level dependencies. This leads to ugly code. The worst case scenario is to have tests which will be removed in a case of code refactoring.
I've checked documentation of a busted https://lunarmodules.github.io/busted/#private - but exporting the helpers for tests does not help at all. I believe this is a good match between testability and a readability of the resulting tests.
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check the tests from https://github.com/tris203/precognition.nvim. They are pretty good. They make tests based on the buffers.
This would be an example for this PR:
----- utility functions
---@param lines string[]
---@return integer bufnr
local create_buf = function(lines)
local bufnr = vim.api.nvim_create_buf(true, true)
vim.api.nvim_buf_set_lines(bufnr, 0, -1, false, lines)
vim.api.nvim_set_current_buf(bufnr)
vim.api.nvim_win_set_cursor(0, { 1, 1 })
return bufnr
end
---@param bufnr integer
---@return string[] lines
local get_buf_lines = function(bufnr)
return vim.api.nvim_buf_get_lines(bufnr, 0, -1, false)
end
----- actual test
it("from_curl (revised)", function()
local bufnr = create_buf({}) -- empty buffer
vim.fn.setreg("+", "curl http://example.com")
UI.from_curl()
local expected = {
[[# curl http://example.com]],
[[GET http://example.com]],
[[]],
[[]],
}
assert.are.same(expected, get_buf_lines(bufnr))
end)
(Those first functions could be on a test utility module)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rcasia amazing approach. Love it more than the heavy mocking. The more I thought about this PR the less I wanted to work on it. This looks like a great compromise between fully functional tests via headless neovim and more approachable unit testing.
return get_buffer() ~= nil | ||
end | ||
|
||
local close_buffer = M._close_buffer | ||
local buffer_exists = M._buffer_exists |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A backward compatibility ... did not want to touch everything for a draft PR
I think we really need more tests and so this is a real good addition. Thanks for your efforts @vyskocilm! |
I'd be happy to continue the PR if needed. |
I'd say your approach is superior, so lets close this PR. There's a little value. |
Nice! Let me continue the work tomorrow. I'll continue this same branch. |
Continued in #266 |
No description provided.