From 70a600c813960d9536ae87c9716034ca0ad1ea31 Mon Sep 17 00:00:00 2001 From: Fredrik Averpil Date: Sun, 14 Jul 2024 00:20:09 +0200 Subject: [PATCH] fix: remove debouncer, use simpler approach --- README.md | 36 +++-- .../features/testify/lookup.lua | 124 +++++++----------- .../features/testify/tree_modification.lua | 74 ++++++++++- lua/neotest-golang/lib/debounce.lua | 65 --------- lua/neotest-golang/lib/init.lua | 1 - lua/neotest-golang/options.lua | 2 - lua/neotest-golang/query.lua | 2 +- tests/go/testify/lookup_spec.lua | 6 +- tests/go/testify/positions_spec.lua | 5 +- tests/unit/options_spec.lua | 4 - 10 files changed, 141 insertions(+), 178 deletions(-) delete mode 100644 lua/neotest-golang/lib/debounce.lua diff --git a/README.md b/README.md index 7b4e70df..442b2063 100644 --- a/README.md +++ b/README.md @@ -75,16 +75,14 @@ You can run `:checkhealth neotest-golang` to review common issues. ## ⚙️ Configuration -| Argument | Default value | Description | -| ------------------------- | ------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `go_test_args` | `{ "-v", "-race", "-count=1" }` | Arguments to pass into `go test`. | -| `dap_go_enabled` | `false` | Leverage [leoluz/nvim-dap-go](https://github.com/leoluz/nvim-dap-go) for debugging tests. | -| `dap_go_opts` | `{}` | Options to pass into `require("dap-go").setup()`. | -| `testify_enabled` | `false` | Enable support for [testify](https://github.com/stretchr/testify) suites. See [here](https://github.com/fredrikaverpil/neotest-golang#testify-suites) for more info. | -| `testify_generate_lookup` | `true` | Automatically re-generate testify lookup when recalculating Neotest tree. | -| `testify_debounce_delay` | `500` | The time to debounce/delay lookup re-generation (in milliseconds). | -| `warn_test_name_dupes` | `true` | Warn about duplicate test names within the same Go package. | -| `warn_test_not_executed` | `true` | Warn if test was not executed. | +| Argument | Default value | Description | +| ------------------------ | ------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `go_test_args` | `{ "-v", "-race", "-count=1" }` | Arguments to pass into `go test`. | +| `dap_go_enabled` | `false` | Leverage [leoluz/nvim-dap-go](https://github.com/leoluz/nvim-dap-go) for debugging tests. | +| `dap_go_opts` | `{}` | Options to pass into `require("dap-go").setup()`. | +| `testify_enabled` | `false` | Enable support for [testify](https://github.com/stretchr/testify) suites. See [here](https://github.com/fredrikaverpil/neotest-golang#testify-suites) for more info. | +| `warn_test_name_dupes` | `true` | Warn about duplicate test names within the same Go package. | +| `warn_test_not_executed` | `true` | Warn if test was not executed. | ### Example configuration: custom `go test` arguments @@ -369,23 +367,21 @@ more information on this. ### Testify suites -> [!WARNING] +> [!WARNING] > This feature comes with some caveats and nuances, which is why it > is not enabled by default. I advise you to only enable this if you need it. There are some real shenaningans going on behind the scenes to make this work. -😅 First, a lookup of "receiver type-to-suite test function" will be created of -all Go test files in your project. Then, the generated Neotest node tree is -modified by mutating private attributes and merging of nodes to avoid +😅 First, an in-memory lookup of "receiver type-to-suite test function" will be +created of all Go test files in your project. Then, the generated Neotest node +tree is modified by mutating private attributes and merging of nodes to avoid duplicates. I'm personally a bit afraid of the maintenance burden of this feature... 🙈 -> [!NOTE] -> Right now, there is no way to update the lookup other than restarting -> Neotest/Neovim. So in case you are implementing a new suite, please restart to -> see the new suites/tests appear in e.g. the summary window. Also, nested tests -> or table tests are not supported. All of this can be remedied at any time. -> Feel free to dig in and open a PR! +> [!NOTE] +> Right now, nested tests and table tests are not supported. All of this +> can be remedied at any time by extending the treesitter queries. Feel free to +> dig in and open a PR! ## 🙏 PRs are welcome diff --git a/lua/neotest-golang/features/testify/lookup.lua b/lua/neotest-golang/features/testify/lookup.lua index 9a3c4eec..58d27446 100644 --- a/lua/neotest-golang/features/testify/lookup.lua +++ b/lua/neotest-golang/features/testify/lookup.lua @@ -1,7 +1,5 @@ --- Lookup table for renaming Neotest namespaces (receiver type to testify suite function). -local options = require("neotest-golang.options") -local lib = require("neotest-golang.lib") local query = require("neotest-golang.features.testify.query") local M = {} @@ -64,88 +62,66 @@ M.query = [[ (identifier)))))) ]] ---- The lookup table store. ---- @type table -local lookup_table = {} +local function create_lookup_manager() + local lookup_table = {} ---- Debouncer for generating the lookup table. ---- @type function -local debounce = lib.debounce.create_debouncer() -local debounced_generate = debounce(function() - return M._generate() -end, options.get().testify_debounce_delay) - ---- Generate the lookup table for testify suites. ---- @return table The generated lookup table -function M.generate() - if options.get().testify_generate_lookup then - local get_result = debounced_generate() - - -- Wait for up to 5 seconds for the result, polling every 100ms. - local max_wait_time = 5000 - local interval_time = 100 - vim.wait(max_wait_time, function() - lookup_table = get_result() - return not vim.tbl_isempty(lookup_table) - end, interval_time) - end + return { + init = function(file_paths) + for _, file_path in ipairs(file_paths) do + lookup_table[file_path] = M.generate_data(file_path) + end + return lookup_table + end, + create = function(file_path) + if not lookup_table[file_path] then + lookup_table[file_path] = M.generate_data(file_path) + end + return lookup_table + end, + get = function() + return lookup_table + end, + clear = function() + lookup_table = {} + end, + } +end - if vim.tbl_isempty(lookup_table) then - vim.notify( - "Warning: generating the lookup timed out.", - vim.log.levels.ERROR - ) - end +-- Create an instance of the lookup manager +local lookup_manager = create_lookup_manager() - return lookup_table -end +--- Public lookup functions. +M.initialize_lookup = lookup_manager.init +M.create_lookup = lookup_manager.create +M.get_lookup = lookup_manager.get +M.clear_lookup = lookup_manager.clear ---- Generate the lookup table for testify suites. +--- Generate the lookup data for the given file. --- @return table The generated lookup table -function M._generate() - vim.notify("Generating testify lookup...", vim.log.levels.INFO) - local cwd = vim.fn.getcwd() - local filepaths = lib.find.go_test_filepaths(cwd) - local lookup = {} +function M.generate_data(file_path) + local data = {} -- First pass: collect all data for the lookup table. - for _, filepath in ipairs(filepaths) do - local matches = query.run_query_on_file(filepath, M.query) - - local package_name = matches.package - and matches.package[1] - and matches.package[1].text - or "unknown" - - lookup[filepath] = { - package = package_name, - replacements = {}, - } - - for i, struct in ipairs(matches.suite_struct or {}) do - local func = matches.test_function[i] - if func then - lookup[filepath].replacements[struct.text] = func.text - end + local matches = query.run_query_on_file(file_path, M.query) + + local package_name = matches.package + and matches.package[1] + and matches.package[1].text + or "unknown" + + data = { + package = package_name, + replacements = {}, + } + + for i, struct in ipairs(matches.suite_struct or {}) do + local func = matches.test_function[i] + if func then + data.replacements[struct.text] = func.text end end - return lookup -end - ---- Get the lookup table for testify suites. ---- @param opts table Options for getting the lookup table ---- @return table The generated lookup table -function M.get(opts) - if options.get().testify_generate_lookup and opts.generate then - return M.generate() - end - return lookup_table -end - ---- Clear the lookup table. -function M.clear() - lookup_table = {} + return data end return M diff --git a/lua/neotest-golang/features/testify/tree_modification.lua b/lua/neotest-golang/features/testify/tree_modification.lua index 614f5f84..1e7a4cb4 100644 --- a/lua/neotest-golang/features/testify/tree_modification.lua +++ b/lua/neotest-golang/features/testify/tree_modification.lua @@ -1,9 +1,14 @@ --- Functions to modify the Neotest tree, for testify suite support. +local options = require("neotest-golang.options") +local lib = require("neotest-golang.lib") local lookup = require("neotest-golang.features.testify.lookup") local M = {} +local lookup_table = lookup.get_lookup() +local ignore_filepaths_during_init = {} + --- Modify the neotest tree, so that testify suites can be executed --- as Neotest namespaces. --- @@ -11,12 +16,30 @@ local M = {} --- type as the Neotest namespace. However, to produce a valid test path, --- this receiver type must be replaced with the testify suite name in the --- Neotest tree. +--- @param file_path string The path to the test file --- @param tree neotest.Tree The original neotest tree --- @return neotest.Tree The modified tree. -function M.modify_neotest_tree(tree) - local lookup_map = lookup.get({ generate = true }) +function M.modify_neotest_tree(file_path, tree) + if vim.tbl_isempty(lookup_table) then + ignore_filepaths_during_init = lib.find.go_test_filepaths(vim.fn.getcwd()) + lookup_table = lookup.initialize_lookup(ignore_filepaths_during_init) + end + + if vim.tbl_contains(ignore_filepaths_during_init, file_path) then + -- some optimization; + -- ignore the first call, as it is handled by the initialization above. + for i, path in ipairs(ignore_filepaths_during_init) do + if path == file_path then + table.remove(ignore_filepaths_during_init, i) + break + end + end + else + -- after initialization, always update the lookup for the given filepath. + lookup_table = lookup.create_lookup(file_path) + end - if not lookup_map then + if not lookup_table then vim.notify( "No lookup found. Could not modify Neotest tree for testify suite support", vim.log.levels.WARN @@ -24,10 +47,11 @@ function M.modify_neotest_tree(tree) return tree end - local modified_tree = M.replace_receiver_with_suite(tree:root(), lookup_map) - local tree_with_merged_namespaces = - M.merge_duplicate_namespaces(modified_tree) - return tree_with_merged_namespaces + local modified_tree = {} + modified_tree = M.replace_receiver_with_suite(tree:root(), lookup_table) + modified_tree = M.merge_duplicate_namespaces(modified_tree) + + return modified_tree end --- Replace receiver methods with their corresponding test suites in the tree. @@ -177,4 +201,40 @@ function M.fix_relationships(n) end end +function M.hash_tree(tree) + local function hash_node(node) + local data = node:data() + local hash_parts = {} + + -- Convert all data fields to strings and add to hash_parts + for k, v in pairs(data) do + if type(v) == "table" then + table.insert(hash_parts, k .. "=" .. vim.inspect(v)) + else + table.insert(hash_parts, k .. "=" .. tostring(v)) + end + end + + -- Sort hash_parts for consistent ordering + table.sort(hash_parts) + + -- Hash children recursively + local children_hashes = {} + for _, child in ipairs(node:children()) do + table.insert(children_hashes, hash_node(child)) + end + table.sort(children_hashes) + + -- Combine node data and children hashes + local combined = table.concat(hash_parts, "|") + .. "#" + .. table.concat(children_hashes, "|") + + -- Use Neovim's built-in hash function + return vim.fn.sha256(combined) + end + + return hash_node(tree) +end + return M diff --git a/lua/neotest-golang/lib/debounce.lua b/lua/neotest-golang/lib/debounce.lua deleted file mode 100644 index eec29287..00000000 --- a/lua/neotest-golang/lib/debounce.lua +++ /dev/null @@ -1,65 +0,0 @@ -local M = {} - -function M.create_debouncer() - local timers = {} - local results = {} - local funcs = {} -- Store the original functions - - return function(func, delay) - if type(func) ~= "function" then - error("First argument to debounce must be a function") - end - - local key = tostring(func) - funcs[key] = func -- Store the function - - return function(...) - local args = { ... } - local callback = nil - - -- Check if the last argument is a callback function - if #args > 0 and type(args[#args]) == "function" then - callback = table.remove(args) - end - - if timers[key] then - vim.fn.timer_stop(timers[key]) - end - - timers[key] = vim.fn.timer_start(delay, function() - if not funcs[key] then - print("Error: Function not found for key: " .. key) - return - end - - local result - local success, err = pcall(function() - if #args > 0 then - result = funcs[key](unpack(args)) - else - result = funcs[key]() - end - end) - - if not success then - print("Error executing function: " .. tostring(err)) - return - end - - results[key] = result - timers[key] = nil - - if callback then - callback(result) - end - end) - - -- Return a function to get the result - return function() - return results[key] - end - end - end -end - -return M diff --git a/lua/neotest-golang/lib/init.lua b/lua/neotest-golang/lib/init.lua index 29d724e6..7ffb8f4f 100644 --- a/lua/neotest-golang/lib/init.lua +++ b/lua/neotest-golang/lib/init.lua @@ -2,7 +2,6 @@ local M = {} M.convert = require("neotest-golang.lib.convert") M.cmd = require("neotest-golang.lib.cmd") -M.debounce = require("neotest-golang.lib.debounce") M.find = require("neotest-golang.lib.find") M.json = require("neotest-golang.lib.json") diff --git a/lua/neotest-golang/options.lua b/lua/neotest-golang/options.lua index ed56da03..d8ce3585 100644 --- a/lua/neotest-golang/options.lua +++ b/lua/neotest-golang/options.lua @@ -9,8 +9,6 @@ local opts = { dap_go_enabled = false, dap_go_opts = {}, testify_enabled = false, - testify_generate_lookup = true, - testify_debounce_delay = 500, warn_test_name_dupes = true, warn_test_not_executed = true, diff --git a/lua/neotest-golang/query.lua b/lua/neotest-golang/query.lua index 43b83826..4b51371a 100644 --- a/lua/neotest-golang/query.lua +++ b/lua/neotest-golang/query.lua @@ -138,7 +138,7 @@ function M.detect_tests(file_path) local tree = lib.treesitter.parse_positions(file_path, query, opts) if options.get().testify_enabled == true then - tree = testify.tree_modification.modify_neotest_tree(tree) + tree = testify.tree_modification.modify_neotest_tree(file_path, tree) end return tree diff --git a/tests/go/testify/lookup_spec.lua b/tests/go/testify/lookup_spec.lua index 9fc26e8b..dd12e878 100644 --- a/tests/go/testify/lookup_spec.lua +++ b/tests/go/testify/lookup_spec.lua @@ -1,6 +1,7 @@ local _ = require("plenary") local options = require("neotest-golang.options") +local lib = require("neotest-golang.lib") local testify = require("neotest-golang.features.testify") describe("Lookup", function() @@ -8,6 +9,7 @@ describe("Lookup", function() -- Arrange options.set({ testify_enabled = true }) -- enable testify local folderpath = vim.loop.cwd() .. "/tests/go" + local filepaths = lib.find.go_test_filepaths(vim.loop.cwd()) local expected_lookup = { [folderpath .. "/positions_test.go"] = { package = "main", @@ -33,10 +35,10 @@ describe("Lookup", function() } -- Act - testify.lookup.generate() -- generate lookup + testify.lookup.initialize_lookup(filepaths) -- generate lookup -- Assert - local lookup = testify.lookup.get() + local lookup = testify.lookup.get_lookup() assert.are.same(vim.inspect(expected_lookup), vim.inspect(lookup)) assert.are.same(expected_lookup, lookup) end) diff --git a/tests/go/testify/positions_spec.lua b/tests/go/testify/positions_spec.lua index fc160b26..d4240009 100644 --- a/tests/go/testify/positions_spec.lua +++ b/tests/go/testify/positions_spec.lua @@ -3,6 +3,7 @@ local _ = require("plenary") local adapter = require("neotest-golang") local options = require("neotest-golang.options") +local lib = require("neotest-golang.lib") local testify = require("neotest-golang.features.testify") local function compareIgnoringKeys(t1, t2, ignoreKeys) @@ -81,8 +82,8 @@ describe("With testify_enabled=true", function() local test_filepath = vim.loop.cwd() .. "/tests/go/testify/positions_test.go" options.set({ testify_enabled = true }) -- enable testify - testify.lookup.generate() -- generate lookup - + local filepaths = lib.find.go_test_filepaths(test_filepath) + testify.lookup.initialize_lookup(filepaths) -- generate lookup local expected = { { id = test_filepath, diff --git a/tests/unit/options_spec.lua b/tests/unit/options_spec.lua index e3188401..96f5aa7d 100644 --- a/tests/unit/options_spec.lua +++ b/tests/unit/options_spec.lua @@ -12,8 +12,6 @@ describe("Options are set up", function() dap_go_enabled = false, dap_go_opts = {}, testify_enabled = false, - testify_generate_lookup = true, - testify_debounce_delay = 500, warn_test_name_dupes = true, warn_test_not_executed = true, @@ -37,8 +35,6 @@ describe("Options are set up", function() dap_go_enabled = false, dap_go_opts = {}, testify_enabled = false, - testify_generate_lookup = true, - testify_debounce_delay = 500, warn_test_name_dupes = true, warn_test_not_executed = true,