feat(tests): Add baseline testing using Plenary's Busted hooks

[levouh]

@cseickel I split this out from the work I am doing on other branches as it seems to be in a relatively good state, and it would be beneficial to have sooner rather than later.

As an overview of what is here:

• Top level Makefile so you can just simply make test and run them as you please (also make format to run stylua)
• Utility function(s) to setup/teardown a "test filesystem" to do whatever you want in
• Verification functions that help with the fact that a lot of the functions in neo-tree are async, so by default if we do something like vim.cmd("NeoTreeFocus") and then try to verify stuff, the verification is almost always going to fail as it happens too soon.

To show what this ends up looking like:

local util = require("tests.helpers.util")
local verify = require("tests.helpers.verify")

describe("Filesystem source command", function()
  local fs = util.setup_test_fs()

  after_each(function()
    util.clear_test_state()
  end)

  it("should reveal the current file in the tree", function()
    local testfile = fs.content[3].abspath
    util.editfile(testfile)

    local start_bufnr = vim.api.nvim_get_current_buf()

    vim.cmd("NeoTree filesystem action=reveal")
    verify.bufnr_is_not(start_bufnr)
    verify.tree_focused()
    verify.tree_node_is(testfile)
  end)

  it("should toggle the reveal-state of the tree", function()
    local testfile = fs.content[3].abspath
    util.editfile(testfile)

    local start_bufnr = vim.api.nvim_get_current_buf()

    vim.cmd("NeoTree filesystem action=reveal toggle=true")
    verify.bufnr_is_not(start_bufnr)
    verify.tree_focused()
    verify.tree_node_is(testfile)

    -- Wait long enough such that the tree _should have_ closed, then assert it is not focused anymore
    vim.cmd("NeoTree filesystem action=reveal toggle=true")
    verify.after(250, function()
      return #vim.api.nvim_tabpage_list_wins(0) == 1 and start_bufnr == vim.api.nvim_get_current_buf()
    end, "Failed to toggle the tree to a closed state with 'action=reveal'")
  end)
  
  util.teardown_test_fs()
end)

A few things to note:

• One thing right now is that the teardown stuff will only make sure that all buffers are wiped, but I am not sure that really is enough "cleanup" to ensure state from one test does not effect another. For example, the state.position metadata is not wiped. It is pretty easy to just wipe that from existence (the entire state object) however I'm not entirely sure what else might need to happen along with that. E.g. bad callbacks, leftover autocommands, etc. so wanted to get your opinion first before going to deep on that. It might be best to just expose a method for testing that "resets" all state (rather than wiping it completely) that any new functionality would need to abide by (meaning providing a way to be reset).
• There is no way to modify the current user configuration, as we only load neo-tree once in the mininit.lua, we could likely want a way to modify the configuration between different tests. Not sure how much of an effort this is at the moment.
• There are no actual examples of the tests here (other than the one above), so if you want I can push up my super WIP examples that use them from Single NeoTree command to interface with different sources #61 I am more than happy to. That branch also has some more unit-style tests as well, while the example above is more functional.
• I have zero clue about Github actions, but you can definitely setup an action to ensure tests pass for merge requests.

This would be the first step towards #77

[levouh]

Should there be a wrapper function for this, such that you can request a file from the tree in a simpler way (that is also more resistant to change in this list)?

Right now you have to:

local fs = utils.setup_test_fs()
local testfile = fs.content[3].abspath

which is obviously not the best thing because as soon as someone messes with the ordering here, all of those references break. One idea might be to give each file a "handle", and access it by the handle, so that a wrapper function can access it like:

local fs = utils.setup_test_ts()
local testfile = fs.get("top_level_file")

which would return a file that suffices the criteria that is being asked for. You could have handles like "empty_dir", "nested_file" and whatnot which might make this a bit better? This turns into a "yo dawg" issue, where you need tests for your tests and so on.

[cseickel]

I think the fs.get(id) method is a good idea. When specifying the nodes, there can be an id property which is optional, and will default to the relative path. makefs can then create a lookup table where the keys are that id and the values are the content items. I think this is simple enough that we don't need to worry about.

[levouh]

@cseickel thoughts on what is being done for cleanup here? It doesn't seem like quite enough as a lot of the internal state is still retained.

[cseickel]

It would be very simple to add a manager._clear_state() method. This is where the decision to store all application state in a single object really pays off.

Another way to accomplish the same thing is by creating a new tab and closing the old one. Of course that in itself could have bugs which need to be tested. I think creating a manager._clear_state() is the best way to go.

[cseickel]

Thanks so much for this! You're right, we definitely need this now. I'll dig into this soon.

[cseickel]

As far as your state concerns:

It is pretty easy to just wipe that from existence (the entire state object) however I'm not entirely sure what else might need to happen along with that. E.g. bad callbacks, leftover autocommands, etc.

I can't think of any issues with just wiping the state. I tried to keep everything as functional as possible with that single state object. Even where events are subscribed, they are based on configuration no state is part of those subscriptions. The only other thing to cleanup is the file watcher subscriptions, but that already has an fs_watch.unwatch_all() method.

we could likely want a way to modify the configuration between different tests. Not sure how much of an effort this is at the moment.

So far, the setup() methods are all idempotent, so we can just call require("neo-tree").setup(config) repeatedly with different options set.

There are no actual examples of the tests here (other than the one above),

I think that's enough, and i can always pop over to your fork and see what you have in the works

I have zero clue about Github actions

Don't worry, I got that one!

[cseickel]

I think the fs.get(id) method is a good idea. When specifying the nodes, there can be an id property which is optional, and will default to the relative path. makefs can then create a lookup table where the keys are that id and the values are the content items. I think this is simple enough that we don't need to worry about.

[cseickel]

It would be very simple to add a manager._clear_state() method. This is where the decision to store all application state in a single object really pays off.

Another way to accomplish the same thing is by creating a new tab and closing the old one. Of course that in itself could have bugs which need to be tested. I think creating a manager._clear_state() is the best way to go.

[cseickel]

I think this is a good to merge, unless you want to add anything after reading my responses. I can add the state cleanup in another commit.

[levouh]

Thanks for the feedback! I'm off for the night so whatever you want to add (either here or elsewhere) is up to you, I'll adapt this to any changes you make.

Let me change a few things tomorrow (might leave the state clearing to you but the other stuff is simple/useful enough) and rebase my current stuff to ensure the "harness" still works before merging, then I can ping you again for a follow up. Obviously no rush though. Thanks again @cseickel!