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

tests(ui): add unit tests for UI functionality #240

Closed
wants to merge 3 commits into from

Conversation

vyskocilm
Copy link
Contributor

No description provided.

@@ -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()
Copy link
Contributor Author

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?

Copy link
Contributor

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)

Copy link
Contributor Author

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
Copy link
Contributor Author

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

@gorillamoe gorillamoe self-assigned this Sep 16, 2024
@gorillamoe gorillamoe added the enhancement New feature or request label Sep 16, 2024
@gorillamoe gorillamoe added help wanted Extra attention is needed question Further information is requested labels Sep 30, 2024
@gorillamoe
Copy link
Member

I think we really need more tests and so this is a real good addition. Thanks for your efforts @vyskocilm!

@rcasia
Copy link
Contributor

rcasia commented Sep 30, 2024

I'd be happy to continue the PR if needed.

@vyskocilm
Copy link
Contributor Author

@rcasia

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.

@rcasia
Copy link
Contributor

rcasia commented Oct 3, 2024

Nice! Let me continue the work tomorrow. I'll continue this same branch.

@rcasia
Copy link
Contributor

rcasia commented Oct 6, 2024

Continued in #266

@gorillamoe gorillamoe closed this Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants