Tests: Handle FM/ReaderUI instances slightly more sanely

* Tear down FM instances properly
* Don't manhandle ReaderUI too much, and document when the tests do
  actively broken shit, like bypassing safeties to open two // ReaderUI
  instances.

frontend/apps/filemanager/filemanager.lua

@@ -544,7 +544,7 @@ function FileManager:init()
     if FileManager.instance == nil then
         logger.dbg("Spinning up new FileManager instance", tostring(self))
     else
-        -- Should never happen, given what we did above...
+        -- Should never happen, given what we did in showFiles...
         logger.err("FileManager instance mismatch! Opened", tostring(self), "while we still have an existing instance:", tostring(FileManager.instance), debug.traceback())
     end
     FileManager.instance = self
@@ -1159,6 +1159,7 @@ function FileManager:getStartWithMenuTable()
     }
 end
 
+--- @note: This is the *only* safe way to instantiate a new FileManager instance!
 function FileManager:showFiles(path, focused_file)
     -- Warn about and close any pre-existing FM instances first...
     if FileManager.instance then

frontend/apps/reader/readerui.lua

@@ -443,7 +443,7 @@ function ReaderUI:init()
     if ReaderUI.instance == nil then
         logger.dbg("Spinning up new ReaderUI instance", tostring(self))
     else
-        -- Should never happen, given what we did above...
+        -- Should never happen, given what we did in (do)showReader...
         logger.err("ReaderUI instance mismatch! Opened", tostring(self), "while we still have an existing instance:", tostring(ReaderUI.instance), debug.traceback())
     end
     ReaderUI.instance = self
@@ -504,6 +504,8 @@ function ReaderUI:onSetupShowReader()
 end
 
 --- @note: Will sanely close existing FileManager/ReaderUI instance for you!
+---        This is the *only* safe way to instantiate a new ReaderUI instance!
+---        (i.e., don't look at the testsuite, which resorts to all kinds of nasty hacks).
 function ReaderUI:showReader(file, provider)
     logger.dbg("show reader ui")
 

spec/unit/filemanager_spec.lua

@@ -18,7 +18,7 @@ describe("FileManager module", function()
             root_path = "../../test",
         }
         UIManager:show(filemanager)
-        UIManager:scheduleIn(1, function() UIManager:close(filemanager) end)
+        UIManager:scheduleIn(1, function() filemanager:onClose() end)
         UIManager:run()
     end)
     it("should show error on non-existent file", function()
@@ -34,6 +34,7 @@ describe("FileManager module", function()
         assert.is_nil(lfs.attributes(tmp_fn))
         filemanager:deleteFile(tmp_fn)
         UIManager.show = old_show
+        filemanager:onClose()
     end)
     it("should not delete settings for non-document file", function()
         local filemanager = FileManager:new{
@@ -62,6 +63,7 @@ describe("FileManager module", function()
         end
         filemanager:deleteFile(tmp_fn)
         UIManager.show = old_show
+        filemanager:onClose()
 
         -- make sure history file exists
         assert.is_nil(lfs.attributes(tmp_fn))
@@ -97,6 +99,7 @@ describe("FileManager module", function()
         end
         filemanager:deleteFile(tmp_fn)
         UIManager.show = old_show
+        filemanager:onClose()
 
         assert.is_nil(lfs.attributes(tmp_fn))
         assert.is_nil(lfs.attributes(tmp_sidecar))

spec/unit/readerbookmark_spec.lua

@@ -24,14 +24,17 @@ describe("ReaderBookmark module", function()
         UIManager:nextTick(function()
             UIManager:close(readerui.highlight.highlight_dialog)
             UIManager:close(readerui)
+            -- We haven't torn it down yet
+            ReaderUI.instance = readerui
         end)
         UIManager:run()
     end
     local function toggler_dogear(readerui)
         readerui.bookmark:onToggleBookmark()
-        --- @todo Replace scheduleIn with nextTick
-        UIManager:scheduleIn(1, function()
+        UIManager:nextTick(function()
             UIManager:close(readerui)
+            -- We haven't torn it down yet
+            ReaderUI.instance = readerui
         end)
         UIManager:run()
     end
@@ -39,6 +42,8 @@ describe("ReaderBookmark module", function()
         UIManager:nextTick(function()
             UIManager:close(readerui.bookmark.bookmark_menu)
             UIManager:close(readerui)
+            -- We haven't torn it down yet
+            ReaderUI.instance = readerui
         end)
         UIManager:run()
     end

spec/unit/readerdictionary_spec.lua

@@ -32,6 +32,8 @@ describe("Readerdictionary module", function()
         UIManager:scheduleIn(1, function()
             UIManager:close(dictionary.dict_window)
             UIManager:close(readerui)
+            -- We haven't torn it down yet
+            ReaderUI.instance = readerui
         end)
         UIManager:run()
         Screen:shot(name)

spec/unit/readerhighlight_spec.lua

@@ -19,6 +19,8 @@ describe("Readerhighlight module", function()
         UIManager:scheduleIn(1, function()
             UIManager:close(readerui.dictionary.dict_window)
             UIManager:close(readerui)
+            -- We haven't torn it down yet
+            ReaderUI.instance = readerui
             UIManager:quit()
         end)
         UIManager:run()
@@ -43,6 +45,8 @@ describe("Readerhighlight module", function()
         UIManager:scheduleIn(1, function()
             UIManager:close(readerui.highlight.highlight_dialog)
             UIManager:close(readerui)
+            -- We haven't torn it down yet
+            ReaderUI.instance = readerui
             UIManager:quit()
         end)
         UIManager:run()
@@ -59,6 +63,8 @@ describe("Readerhighlight module", function()
         UIManager:nextTick(function()
             UIManager:close(readerui.highlight.edit_highlight_dialog)
             UIManager:close(readerui)
+            -- We haven't torn it down yet
+            ReaderUI.instance = readerui
             UIManager:quit()
         end)
         UIManager:run()
@@ -183,6 +189,8 @@ describe("Readerhighlight module", function()
                 readerui.highlight:clear()
                 readerui.document.configurable.text_wrap = 0
                 UIManager:close(readerui)  -- close to flush settings
+                -- We haven't torn it down yet
+                ReaderUI.instance = readerui
             end)
             it("should highlight single word", function()
                 highlight_single_word(readerui, Geom:new{ x = 260, y = 70 })
@@ -273,6 +281,8 @@ describe("Readerhighlight module", function()
                 readerui.highlight:clear()
                 readerui.document.configurable.text_wrap = 0
                 UIManager:close(readerui)  -- close to flush settings
+                -- We haven't torn it down yet
+                ReaderUI.instance = readerui
             end)
             it("should highlight single word", function()
                 highlight_single_word(readerui, Geom:new{ x = 260, y = 70 })

spec/unit/readerpaging_spec.lua

@@ -28,7 +28,11 @@ describe("Readerpaging module", function()
         it("should emit EndOfBook event at the end", function()
             UIManager:quit()
             UIManager:show(readerui)
-            UIManager:nextTick(function() UIManager:close(readerui) end)
+            UIManager:nextTick(function()
+                UIManager:close(readerui)
+                -- We haven't torn it down yet
+                ReaderUI.instance = readerui
+            end)
             UIManager:run()
             readerui:handleEvent(Event:new("SetScrollMode", false))
             readerui.zooming:setZoomMode("pageheight")
@@ -65,7 +69,11 @@ describe("Readerpaging module", function()
         it("should emit EndOfBook event at the end", function()
             UIManager:quit()
             UIManager:show(readerui)
-            UIManager:nextTick(function() UIManager:close(readerui) end)
+            UIManager:nextTick(function()
+                UIManager:close(readerui)
+                -- We haven't torn it down yet
+                ReaderUI.instance = readerui
+            end)
             UIManager:run()
             paging.page_positions = {}
             readerui:handleEvent(Event:new("SetScrollMode", true))
@@ -84,6 +92,7 @@ describe("Readerpaging module", function()
 
         it("should scroll backward on the first page without crash", function()
             local sample_djvu = "spec/front/unit/data/djvu3spec.djvu"
+            -- Unsafe second // ReaderUI instance!
             local tmp_readerui = ReaderUI:new{
                 dimen = Screen:getSize(),
                 document = DocumentRegistry:openDocument(sample_djvu),
@@ -91,10 +100,13 @@ describe("Readerpaging module", function()
             tmp_readerui.paging:onScrollPanRel(-100)
             tmp_readerui:closeDocument()
             tmp_readerui:onClose()
+            -- Restore the ref to the original ReaderUI instance
+            ReaderUI.instance = readerui
         end)
 
         it("should scroll forward on the last page without crash", function()
             local sample_djvu = "spec/front/unit/data/djvu3spec.djvu"
+            -- Unsafe second // ReaderUI instance!
             local tmp_readerui = ReaderUI:new{
                 dimen = Screen:getSize(),
                 document = DocumentRegistry:openDocument(sample_djvu),
@@ -106,6 +118,8 @@ describe("Readerpaging module", function()
             paging:onScrollPanRel(120)
             tmp_readerui:closeDocument()
             tmp_readerui:onClose()
+            -- Restore the ref to the original ReaderUI instance
+            ReaderUI.instance = readerui
         end)
     end)
 end)

spec/unit/readerrolling_spec.lua

@@ -80,6 +80,7 @@ describe("Readerrolling module", function()
 
         it("should emit EndOfBook event at the end sample txt", function()
             local sample_txt = "spec/front/unit/data/sample.txt"
+            -- Unsafe second // ReaderUI instance!
             local txt_readerui = ReaderUI:new{
                 dimen = Screen:getSize(),
                 document = DocumentRegistry:openDocument(sample_txt),
@@ -108,6 +109,8 @@ describe("Readerrolling module", function()
             readerui.onEndOfBook = nil
             txt_readerui:closeDocument()
             txt_readerui:onClose()
+            -- Restore the ref to the original ReaderUI instance
+            ReaderUI.instance = readerui
         end)
     end)
 
@@ -207,14 +210,14 @@ describe("Readerrolling module", function()
             end
             local test_book = "spec/front/unit/data/sample.txt"
             require("docsettings"):open(test_book):purge()
+            readerui:closeDocument()
+            readerui:onClose()
             local tmp_readerui = ReaderUI:new{
                 document = DocumentRegistry:openDocument(test_book),
             }
             ReaderView.onPageUpdate = saved_handler
             tmp_readerui:closeDocument()
             tmp_readerui:onClose()
-            readerui:closeDocument()
-            readerui:onClose()
         end)
     end)
 end)

spec/unit/readerui_spec.lua

@@ -30,7 +30,11 @@ describe("Readerui module", function()
     it("should show reader", function()
         UIManager:quit()
         UIManager:show(readerui)
-        UIManager:scheduleIn(1, function() UIManager:close(readerui) end)
+        UIManager:scheduleIn(1, function()
+            UIManager:close(readerui)
+            -- We haven't torn it down yet
+            ReaderUI.instance = readerui
+        end)
         UIManager:run()
     end)
     it("should close document", function()
@@ -38,10 +42,13 @@ describe("Readerui module", function()
         assert(readerui.document == nil)
         readerui:onClose()
     end)
-    it("should not reset running_instance by mistake", function()
-        ReaderUI:doShowReader(sample_epub)
+    it("should not reset ReaderUI.instance by mistake", function()
+        ReaderUI:doShowReader(sample_epub) -- spins up a new, sane instance
         local new_readerui = ReaderUI:_getRunningInstance()
         assert.is.truthy(new_readerui.document)
+        -- This *will* trip:
+        -- * A pair of ReaderUI instance mimsatch warnings (on open/close) because it bypasses the safety of doShowReader!
+        -- * A refcount warning from DocumentRegistry, because bypassinf the safeties means that two different instances opened the same Document.
         ReaderUI:new{
             dimen = Screen:getSize(),
             document = DocumentRegistry:openDocument(sample_epub)

spec/unit/screenshoter_spec.lua

@@ -28,7 +28,11 @@ describe("ReaderScreenshot module", function()
         readerui:handleEvent(Event:new("SetRotationMode", Screen.ORIENTATION_PORTRAIT))
         UIManager:quit()
         UIManager:show(readerui)
-        UIManager:scheduleIn(1, function() UIManager:close(readerui) end)
+        UIManager:scheduleIn(1, function()
+            UIManager:close(readerui)
+            -- We haven't torn it down yet
+            ReaderUI.instance = readerui
+        end)
         UIManager:run()
         readerui.screenshot:onScreenshot(name)
         assert.truthy(lfs.attributes(name, "mode"))
@@ -40,7 +44,11 @@ describe("ReaderScreenshot module", function()
         readerui:handleEvent(Event:new("SetRotationMode", Screen.ORIENTATION_LANDSCAPE))
         UIManager:quit()
         UIManager:show(readerui)
-        UIManager:scheduleIn(2, function() UIManager:close(readerui) end)
+        UIManager:scheduleIn(2, function()
+            UIManager:close(readerui)
+            -- We haven't torn it down yet
+            ReaderUI.instance = readerui
+        end)
         UIManager:run()
         readerui.screenshot:onScreenshot(name)
         assert.truthy(lfs.attributes(name, "mode"))