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

fix(package): don't call vim API functions inside fast event #730

Merged
merged 2 commits into from
Dec 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions lua/mason-core/package/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,10 @@ function Package:new_handle()
local handle = InstallationHandle.new(self)
self.handle = handle

-- First emit a private autocmd via the native event bus. This is to enable some internal perf improvements (helps avoid loading some Lua modules).
if vim.fn.has "nvim-0.8.0" == 1 then
vim.api.nvim_exec_autocmds("User", { pattern = "__MasonPackageHandle", data = self.name })
else
vim.api.nvim_exec_autocmds("User", { pattern = "__MasonPackageHandle" })
end
-- Ideally we'd decouple this and leverage Mason's event system, but to allow loading as little as possible during
-- setup (i.e. not load modules related to Mason's event system) of the mason.nvim plugin we explicitly call into
-- terminator here.
require("mason-core.terminator").register(handle)

self:emit("handle", handle)
registry:emit("package:handle", self, handle)
Expand Down
18 changes: 8 additions & 10 deletions lua/mason/terminator.lua → lua/mason-core/terminator.lua
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
local registry = require "mason-registry"
local a = require "mason-core.async"

-- Hasta la vista, baby.
Expand Down Expand Up @@ -56,15 +55,14 @@ end

local active_handles = {}

function M.setup()
registry:on("package:handle", function(_, handle)
if handle:is_closed() then
return
end
active_handles[handle] = true
handle:once("closed", function()
active_handles[handle] = nil
end)
---@parma handle InstallHandle
function M.register(handle)
if handle:is_closed() then
return
end
active_handles[handle] = true
handle:once("closed", function()
active_handles[handle] = nil
end)
end

Expand Down
11 changes: 1 addition & 10 deletions lua/mason/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,9 @@ local platform = require "mason-core.platform"
local M = {}

local function setup_autocmds()
-- lazily set up terminator
vim.api.nvim_create_autocmd("User", {
pattern = "__MasonPackageHandle", -- private autocmd specific for this very use case
callback = function()
require("mason.terminator").setup()
end,
once = true,
})

vim.api.nvim_create_autocmd("VimLeavePre", {
callback = function()
require("mason.terminator").terminate(5000)
require("mason-core.terminator").terminate(5000)
end,
once = true,
})
Expand Down
1 change: 0 additions & 1 deletion tests/mason-core/managers/pip3_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ describe("pip3 manager", function()
local handle = InstallHandleGenerator "dummy"
local ctx = InstallContextGenerator(handle)
installer.run_installer(ctx, pip3.packages { "package" })
vim.pretty_print(ctx.spawn.python)
assert.spy(ctx.spawn.python).was_called(2)
assert.spy(ctx.spawn.python).was_called_with {
"-m",
Expand Down
144 changes: 89 additions & 55 deletions tests/mason-core/package/package_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@ local spy = require "luassert.spy"
local mock = require "luassert.mock"
local stub = require "luassert.stub"
local match = require "luassert.match"
local a = require "mason-core.async"
local Pkg = require "mason-core.package"
local registry = require "mason-registry"
local Result = require "mason-core.result"

describe("package", function()
before_each(function()
registry.get_package("dummy"):uninstall()
package.loaded["dummy_package"] = nil
end)

Expand Down Expand Up @@ -78,7 +79,6 @@ describe("package", function()
end)

it("should create new handle", function()
---@type Package
local dummy = registry.get_package "dummy"
-- yo dawg
local handle_handler = spy.new()
Expand All @@ -89,7 +89,6 @@ describe("package", function()
end)

it("should not create new handle if one already exists", function()
---@type Package
local dummy = registry.get_package "dummy"
dummy.handle = mock.new {
is_closed = mockx.returns(false),
Expand All @@ -103,57 +102,92 @@ describe("package", function()
assert.spy(handle_handler).was_called(0)
end)

it("should successfully install package", function()
local installer = require "mason-core.installer"
stub(installer, "execute")
installer.execute.returns(Result.success "Yay!")
---@type Package
local dummy = registry.get_package "dummy"
local package_install_success_handler = spy.new()
local package_install_failed_handler = spy.new()
local install_success_handler = spy.new()
local install_failed_handler = spy.new()
registry:once("package:install:success", package_install_success_handler)
registry:once("package:install:failed", package_install_failed_handler)
dummy:once("install:success", install_success_handler)
dummy:once("install:failed", install_failed_handler)

local handle = dummy:install { version = "1337" }

assert.spy(installer.execute).was_called(1)
assert.spy(installer.execute).was_called_with(match.is_ref(handle), { requested_version = "1337" })
assert.spy(install_success_handler).was_called(1)
assert.spy(install_success_handler).was_called_with(match.is_ref(handle))
assert.spy(package_install_success_handler).was_called(1)
assert.spy(package_install_success_handler).was_called_with(match.is_ref(dummy), match.is_ref(handle))
assert.spy(package_install_failed_handler).was_called(0)
assert.spy(install_failed_handler).was_called(0)
end)
it(
"should successfully install package",
async_test(function()
local dummy = registry.get_package "dummy"
local package_install_success_handler = spy.new()
local package_install_failed_handler = spy.new()
local install_success_handler = spy.new()
local install_failed_handler = spy.new()
registry:once("package:install:success", package_install_success_handler)
registry:once("package:install:failed", package_install_failed_handler)
dummy:once("install:success", install_success_handler)
dummy:once("install:failed", install_failed_handler)

local handle = dummy:install { version = "1337" }

assert.wait_for(function()
assert.is_true(handle:is_closed())
assert.is_true(dummy:is_installed())
end)

it("should fail to install package", function()
local installer = require "mason-core.installer"
stub(installer, "execute")
installer.execute.returns(Result.failure "Oh no.")
---@type Package
local dummy = registry.get_package "dummy"
local package_install_success_handler = spy.new()
local package_install_failed_handler = spy.new()
local install_success_handler = spy.new()
local install_failed_handler = spy.new()
registry:once("package:install:success", package_install_success_handler)
registry:once("package:install:failed", package_install_failed_handler)
dummy:once("install:success", install_success_handler)
dummy:once("install:failed", install_failed_handler)

local handle = dummy:install { version = "1337" }

assert.spy(installer.execute).was_called(1)
assert.spy(installer.execute).was_called_with(match.is_ref(handle), { requested_version = "1337" })
assert.spy(install_failed_handler).was_called(1)
assert.spy(install_failed_handler).was_called_with(match.is_ref(handle))
assert.spy(package_install_failed_handler).was_called(1)
assert.spy(package_install_failed_handler).was_called_with(match.is_ref(dummy), match.is_ref(handle))
assert.spy(package_install_success_handler).was_called(0)
assert.spy(install_success_handler).was_called(0)
end)
assert.spy(install_success_handler).was_called(1)
assert.spy(install_success_handler).was_called_with(match.is_ref(handle))
assert.spy(package_install_success_handler).was_called(1)
assert.spy(package_install_success_handler).was_called_with(match.is_ref(dummy), match.is_ref(handle))
assert.spy(package_install_failed_handler).was_called(0)
assert.spy(install_failed_handler).was_called(0)
end)
)

it(
"should fail to install package",
async_test(function()
local dummy = registry.get_package "dummy"
stub(dummy.spec, "install")
dummy.spec.install.invokes(function()
error "I simply refuse to be installed."
end)
local package_install_success_handler = spy.new()
local package_install_failed_handler = spy.new()
local install_success_handler = spy.new()
local install_failed_handler = spy.new()
registry:once("package:install:success", package_install_success_handler)
registry:once("package:install:failed", package_install_failed_handler)
dummy:once("install:success", install_success_handler)
dummy:once("install:failed", install_failed_handler)

local handle = dummy:install { version = "1337" }

assert.wait_for(function()
assert.is_true(handle:is_closed())
assert.is_false(dummy:is_installed())
end)

assert.spy(install_failed_handler).was_called(1)
assert.spy(install_failed_handler).was_called_with(match.is_ref(handle))
assert.spy(package_install_failed_handler).was_called(1)
assert.spy(package_install_failed_handler).was_called_with(match.is_ref(dummy), match.is_ref(handle))
assert.spy(package_install_success_handler).was_called(0)
assert.spy(install_success_handler).was_called(0)
end)
)

it(
"should be able to start package installation outside of main loop",
async_test(function()
local dummy = registry.get_package "dummy"

-- Move outside the main loop
a.wait(function(resolve)
local timer = vim.loop.new_timer()
timer:start(0, 0, function()
timer:close()
resolve()
end)
end)

assert.is_true(vim.in_fast_event())

local handle = assert.is_not.has_error(function()
return dummy:install()
end)

assert.wait_for(function()
assert.is_true(handle:is_closed())
assert.is_true(dummy:is_installed())
end)
end)
)
end)
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,11 @@ local match = require "luassert.match"
local spy = require "luassert.spy"
local a = require "mason-core.async"
local registry = require "mason-registry"
local terminator = require "mason.terminator"
local terminator = require "mason-core.terminator"
local _ = require "mason-core.functional"
local InstallHandle = require "mason-core.installer.handle"

describe("terminator", function()
before_each(function()
terminator.setup()
end)

it(
"should terminate all active handles on nvim exit",
async_test(function()
Expand Down
3 changes: 3 additions & 0 deletions vim.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,6 @@ globals:
any: true
assert.snapshot:
args: []
assert.is_not.has_error:
args:
- type: function