Skip to content

Commit

Permalink
test(set_page): finish testing this for now
Browse files Browse the repository at this point in the history
Also confirmed that all of the mentioned bugs were introduced by the
asserts I added, and were not inherited from sfinv. They've all been
fixed.
  • Loading branch information
Lazerbeak12345 committed Nov 7, 2023
1 parent 0dc623f commit c4828db
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 7 deletions.
6 changes: 3 additions & 3 deletions api.lua
Original file line number Diff line number Diff line change
Expand Up @@ -233,16 +233,16 @@ end
function sway.set_page(player, pagename)
local context = sway.get_or_create_context(player)
local oldpage = sway.pages[context.page]
if oldpage and oldpage.on_leave then
oldpage:on_leave(player, context)
end
local type_pagename = type(pagename)
assert(
type_pagename == "string",
"[sway] set_page: expected a string for the page name. Got a '" .. type_pagename .. "'"
)
local page = sway.pages[pagename]
assert(page, "[sway] set_page: Page not found: '".. pagename .."'")
if oldpage and oldpage.on_leave then
oldpage:on_leave(player, context)
end
context.page = pagename
if page.on_enter then
page:on_enter(player, context)
Expand Down
25 changes: 21 additions & 4 deletions spec/sway_inv_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,6 @@ describe("pages", function ()
sway.set_page({ get_player_name = ident"playername" }, {})
end, "[sway] set_page: expected a string for the page name. Got a 'table'")
end)
-- TODO This test actually found a bug in sfinv... It should assert before setting context.page
it("gets the current page, sets to the new page and asserts if the newpage is invalid", function ()
local old_pages = sway.pages
local old_get_or_create_context = sway.get_or_create_context
Expand Down Expand Up @@ -319,9 +318,27 @@ describe("pages", function ()
assert.same({page = "real:page"}, ctx, "context")
assert.same({{old_page, player, ctx}}, ol_calls, "ol_calls")
end)
-- BUG: ⬇️ this is a bug in both sfinv and sway. Could cause the dynamically-typed equivelant of a double-free or a
-- use-after-free ⬇️
pending"on_leave is not called if the page is not found"
it("on_leave is not called if the page is not found", function ()
local old_pages = sway.pages
local old_get_or_create_context = sway.get_or_create_context
local ctx = {page = "old:page"}
local ol_calls = {}
local function on_leave(...)
ol_calls[#ol_calls+1] = {...}
end
local old_page = { on_leave = on_leave }
sway.pages = { ["old:page"]=old_page }
sway.get_or_create_context = function ()
return ctx
end
assert.has_error(function ()
sway.set_page({}, "fake:page")
end, "[sway] set_page: Page not found: 'fake:page'")
assert.same({ page = "old:page" }, ctx, "context")
assert.same({}, ol_calls, "ol_calls")
sway.get_or_create_context = old_get_or_create_context
sway.pages = old_pages
end)
it("if there's an on_enter function it calls it", function ()
local old_get_or_create_context = sway.get_or_create_context
local old_pages = sway.pages
Expand Down

0 comments on commit c4828db

Please sign in to comment.