From c23c422f624abb87798540abc5a9e3f04b3e880a Mon Sep 17 00:00:00 2001 From: NiLuJe Date: Sun, 13 Aug 2023 11:03:12 +0200 Subject: [PATCH] Test: Fiw readerhighlight test With actual ReaderUI teardowns, and fix the overlapping highlight handling, so we can drop the nocov flags --- frontend/apps/reader/modules/readerview.lua | 5 +- spec/unit/readerhighlight_spec.lua | 255 +++++++++++++------- 2 files changed, 163 insertions(+), 97 deletions(-) diff --git a/frontend/apps/reader/modules/readerview.lua b/frontend/apps/reader/modules/readerview.lua index a904403e2..317355b65 100644 --- a/frontend/apps/reader/modules/readerview.lua +++ b/frontend/apps/reader/modules/readerview.lua @@ -1111,10 +1111,7 @@ end function ReaderView:onCloseWidget() -- Stop any pending HintPage event UIManager:unschedule(self.emitHintPageEvent) - --- @fixme: The awful readerhighlight_spec test *relies* on this pointer being left dangling... - if not self.ui._testsuite then - self.emitHintPageEvent = nil - end + self.emitHintPageEvent = nil end function ReaderView:onReaderReady() diff --git a/spec/unit/readerhighlight_spec.lua b/spec/unit/readerhighlight_spec.lua index 3808b76c0..7a3c47ceb 100644 --- a/spec/unit/readerhighlight_spec.lua +++ b/spec/unit/readerhighlight_spec.lua @@ -12,7 +12,8 @@ describe("Readerhighlight module", function() UIManager = require("ui/uimanager") end) - local function highlight_single_word(readerui, pos0) + local function highlight_single_word(pos0) + local readerui = ReaderUI.instance local s = spy.on(readerui.languagesupport, "improveWordSelection") readerui.highlight:onHold(nil, { pos = pos0 }) @@ -28,13 +29,12 @@ 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() end - local function highlight_text(readerui, pos0, pos1) + local function highlight_text(pos0, pos1) + local readerui = ReaderUI.instance readerui.highlight:onHold(nil, { pos = pos0 }) readerui.highlight:onHoldPan(nil, { pos = pos1 }) local next_slot @@ -54,13 +54,16 @@ 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() end - local function tap_highlight_text(readerui, pos0, pos1, pos2) + local function tap_highlight_text(pos0, pos1, pos2) + local readerui = ReaderUI.instance + -- Check the actual call chain, instead of relying on the actual internal highlight_dialog object directly... + -- Besides being less nutty, this will work for overlapping highlights... + local s = spy.on(readerui.highlight, "showChooseHighlightDialog") + readerui.highlight:onHold(nil, { pos = pos0 }) readerui.highlight:onHoldPan(nil, { pos = pos1 }) readerui.highlight:onHoldRelease() @@ -68,12 +71,15 @@ describe("Readerhighlight module", function() readerui.highlight:clear() UIManager:close(readerui.highlight.highlight_dialog) readerui.highlight:onTap(nil, { pos = pos2 }) - assert.truthy(readerui.highlight.edit_highlight_dialog) + if not readerui.highlight.edit_highlight_dialog then + -- Take an up-to-date screenshot if this step failed, it's probably because we found overlapping HLs + UIManager:forceRePaint() + Screen:shot("screenshots/tap_highlight_text_overlapping_highlights.png") + end + assert.spy(s).was_called() 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() @@ -81,23 +87,18 @@ describe("Readerhighlight module", function() describe("highlight for EPUB documents", function() local page = 10 - local readerui, selection_spy - setup(function() + before_each(function() + UIManager:quit() + local sample_epub = "spec/front/unit/data/juliet.epub" - readerui = ReaderUI:new{ + local readerui = ReaderUI:new{ dimen = Screen:getSize(), document = DocumentRegistry:openDocument(sample_epub), } - selection_spy = spy.on(readerui.languagesupport, "improveWordSelection") - end) - teardown(function() - readerui:closeDocument() - readerui:onClose() - end) - before_each(function() - UIManager:quit() - readerui.rolling:onGotoPage(page) + local selection_spy = spy.on(readerui.languagesupport, "improveWordSelection") + UIManager:show(readerui) + readerui.rolling:onGotoPage(page) selection_spy:clear() --- @fixme HACK: Mock UIManager:run x and y for readerui.dimen --- @todo Refactor readerview's dimen handling so we can get rid of @@ -105,25 +106,38 @@ describe("Readerhighlight module", function() readerui:paintTo(Screen.bb, 0, 0) end) after_each(function() - readerui.highlight:clear() + local readerui = ReaderUI.instance + + if readerui then + readerui.highlight:clear() + readerui:closeDocument() + readerui:onClose() + end end) it("should highlight single word", function() - highlight_single_word(readerui, Geom:new{ x = 400, y = 70 }) + local readerui = ReaderUI.instance + local selection_spy = spy.on(readerui.languagesupport, "improveWordSelection") + + highlight_single_word(Geom:new{ x = 400, y = 70 }) Screen:shot("screenshots/reader_highlight_single_word_epub.png") assert.spy(selection_spy).was_called() assert.truthy(readerui.view.highlight.saved[page]) end) it("should highlight text", function() - highlight_text(readerui, - Geom:new{ x = 400, y = 110 }, + local readerui = ReaderUI.instance + local selection_spy = spy.on(readerui.languagesupport, "improveWordSelection") + + highlight_text(Geom:new{ x = 400, y = 110 }, Geom:new{ x = 400, y = 170 }) Screen:shot("screenshots/reader_highlight_text_epub.png") assert.spy(selection_spy).was_called() assert.truthy(readerui.view.highlight.saved[page]) end) it("should response on tap gesture", function() - tap_highlight_text(readerui, - Geom:new{ x = 130, y = 100 }, + local readerui = ReaderUI.instance + local selection_spy = spy.on(readerui.languagesupport, "improveWordSelection") + + tap_highlight_text(Geom:new{ x = 130, y = 100 }, Geom:new{ x = 350, y = 395 }, Geom:new{ x = 80, y = 265 }) Screen:shot("screenshots/reader_tap_highlight_text_epub.png") @@ -132,132 +146,160 @@ describe("Readerhighlight module", function() end) describe("highlight for PDF documents in page mode", function() - local readerui - setup(function() - local sample_pdf = "spec/front/unit/data/sample.pdf" - readerui = ReaderUI:new{ - dimen = Screen:getSize(), - document = DocumentRegistry:openDocument(sample_pdf), - _testsuite = true, - } - readerui:handleEvent(Event:new("SetScrollMode", false)) - end) - teardown(function() - readerui:closeDocument() - readerui:onClose() - end) + local sample_pdf = "spec/front/unit/data/sample.pdf" describe("for scanned page with text layer", function() before_each(function() UIManager:quit() + + local readerui = ReaderUI:new{ + dimen = Screen:getSize(), + document = DocumentRegistry:openDocument(sample_pdf), + } + readerui:handleEvent(Event:new("SetScrollMode", false)) + UIManager:show(readerui) readerui.paging:onGotoPage(10) + -- We want up-to-date screenshots + readerui:paintTo(Screen.bb, 0, 0) end) after_each(function() - readerui.highlight:clear() + local readerui = ReaderUI.instance + + if readerui then + readerui.highlight:clear() + readerui:closeDocument() + readerui:onClose() + end end) - it("should response on tap gesture #nocov", function() - tap_highlight_text(readerui, - Geom:new{ x = 260, y = 70 }, + it("should response on tap gesture", function() + tap_highlight_text(Geom:new{ x = 260, y = 70 }, Geom:new{ x = 260, y = 150 }, Geom:new{ x = 280, y = 110 }) Screen:shot("screenshots/reader_tap_highlight_text_pdf.png") end) it("should highlight single word", function() - highlight_single_word(readerui, Geom:new{ x = 260, y = 70 }) + highlight_single_word(Geom:new{ x = 260, y = 70 }) Screen:shot("screenshots/reader_highlight_single_word_pdf.png") end) it("should highlight text", function() - highlight_text(readerui, Geom:new{ x = 260, y = 170 }, Geom:new{ x = 260, y = 250 }) + highlight_text(Geom:new{ x = 260, y = 170 }, Geom:new{ x = 260, y = 250 }) Screen:shot("screenshots/reader_highlight_text_pdf.png") end) end) describe("for scanned page without text layer", function() before_each(function() UIManager:quit() + + local readerui = ReaderUI:new{ + dimen = Screen:getSize(), + document = DocumentRegistry:openDocument(sample_pdf), + } + readerui:handleEvent(Event:new("SetScrollMode", false)) + UIManager:show(readerui) readerui.paging:onGotoPage(28) + readerui:paintTo(Screen.bb, 0, 0) end) after_each(function() - readerui.highlight:clear() + local readerui = ReaderUI.instance + + if readerui then + readerui.highlight:clear() + readerui:closeDocument() + readerui:onClose() + end end) - it("should respond to tap gesture #nocov", function() - tap_highlight_text(readerui, Geom:new{ x = 260, y = 70 }, Geom:new{ x = 260, y = 150 }, Geom:new{ x = 280, y = 110 }) + it("should respond to tap gesture", function() + tap_highlight_text(Geom:new{ x = 260, y = 70 }, Geom:new{ x = 260, y = 150 }, Geom:new{ x = 250, y = 75 }) Screen:shot("screenshots/reader_tap_highlight_text_pdf_scanned.png") end) it("should highlight single word", function() - highlight_single_word(readerui, Geom:new{ x = 260, y = 70 }) + highlight_single_word(Geom:new{ x = 260, y = 70 }) Screen:shot("screenshots/reader_highlight_single_word_pdf_scanned.png") end) it("should highlight text", function() - highlight_text(readerui, Geom:new{ x = 260, y = 70 }, Geom:new{ x = 260, y = 150 }) + highlight_text(Geom:new{ x = 260, y = 70 }, Geom:new{ x = 260, y = 150 }) Screen:shot("screenshots/reader_highlight_text_pdf_scanned.png") end) end) describe("for reflowed page", function() before_each(function() UIManager:quit() + + local readerui = ReaderUI:new{ + dimen = Screen:getSize(), + document = DocumentRegistry:openDocument(sample_pdf), + } + readerui:handleEvent(Event:new("SetScrollMode", false)) readerui.document.configurable.text_wrap = 1 + readerui:handleEvent(Event:new("ReflowUpdated")) + UIManager:show(readerui) readerui.paging:onGotoPage(31) + readerui:paintTo(Screen.bb, 0, 0) end) after_each(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 + local readerui = ReaderUI.instance + + if readerui then + readerui.highlight:clear() + readerui.document.configurable.text_wrap = 0 + readerui:handleEvent(Event:new("ReflowUpdated")) + readerui:closeDocument() + readerui:onClose() + end end) - it("should response on tap gesture #nocov", function() - tap_highlight_text(readerui, Geom:new{ x = 260, y = 70 }, Geom:new{ x = 260, y = 150 }, Geom:new{ x = 280, y = 110 }) + it("should respond to tap gesture", function() + tap_highlight_text(Geom:new{ x = 260, y = 70 }, Geom:new{ x = 260, y = 150 }, Geom:new{ x = 250, y = 75 }) Screen:shot("screenshots/reader_tap_highlight_text_pdf_reflowed.png") end) it("should highlight single word", function() - highlight_single_word(readerui, Geom:new{ x = 260, y = 70 }) + highlight_single_word(Geom:new{ x = 260, y = 70 }) Screen:shot("screenshots/reader_highlight_single_word_pdf_reflowed.png") end) it("should highlight text", function() - highlight_text(readerui, Geom:new{ x = 260, y = 70 }, Geom:new{ x = 260, y = 150 }) + highlight_text(Geom:new{ x = 260, y = 70 }, Geom:new{ x = 260, y = 150 }) Screen:shot("screenshots/reader_highlight_text_pdf_reflowed.png") end) end) end) describe("highlight for PDF documents in scroll mode", function() - local readerui - setup(function() - local sample_pdf = "spec/front/unit/data/sample.pdf" - readerui = ReaderUI:new{ - dimen = Screen:getSize(), - document = DocumentRegistry:openDocument(sample_pdf), - _testsuite = true, - } - readerui:handleEvent(Event:new("SetScrollMode", true)) - end) - teardown(function() - readerui:closeDocument() - readerui:onClose() - end) + local sample_pdf = "spec/front/unit/data/sample.pdf" describe("for scanned page with text layer", function() before_each(function() UIManager:quit() + + local readerui = ReaderUI:new{ + dimen = Screen:getSize(), + document = DocumentRegistry:openDocument(sample_pdf), + } + readerui:handleEvent(Event:new("SetScrollMode", true)) + UIManager:show(readerui) readerui.paging:onGotoPage(10) readerui.zooming:setZoomMode("contentwidth") + readerui:paintTo(Screen.bb, 0, 0) end) after_each(function() - readerui.highlight:clear() + local readerui = ReaderUI.instance + + if readerui then + readerui.highlight:clear() + readerui:closeDocument() + readerui:onClose() + end end) it("should highlight single word", function() - highlight_single_word(readerui, Geom:new{ x = 260, y = 70 }) + highlight_single_word(Geom:new{ x = 260, y = 70 }) Screen:shot("screenshots/reader_highlight_single_word_pdf_scroll.png") end) it("should highlight text", function() - highlight_text(readerui, Geom:new{ x = 260, y = 170 }, Geom:new{ x = 260, y = 250 }) + highlight_text(Geom:new{ x = 260, y = 170 }, Geom:new{ x = 260, y = 250 }) Screen:shot("screenshots/reader_highlight_text_pdf_scroll.png") end) it("should response on tap gesture", function() - tap_highlight_text(readerui, - Geom:new{ x = 260, y = 70 }, + tap_highlight_text(Geom:new{ x = 260, y = 70 }, Geom:new{ x = 260, y = 150 }, Geom:new{ x = 280, y = 110 }) Screen:shot("screenshots/reader_tap_highlight_text_pdf_scroll.png") @@ -266,50 +308,77 @@ describe("Readerhighlight module", function() describe("for scanned page without text layer", function() before_each(function() UIManager:quit() + + local readerui = ReaderUI:new{ + dimen = Screen:getSize(), + document = DocumentRegistry:openDocument(sample_pdf), + } + readerui:handleEvent(Event:new("SetScrollMode", true)) + UIManager:show(readerui) readerui.paging:onGotoPage(28) readerui.zooming:setZoomMode("contentwidth") + readerui:paintTo(Screen.bb, 0, 0) end) after_each(function() - readerui.highlight:clear() + local readerui = ReaderUI.instance + + if readerui then + readerui.highlight:clear() + readerui:closeDocument() + readerui:onClose() + end end) it("should highlight single word", function() - highlight_single_word(readerui, Geom:new{ x = 260, y = 70 }) + highlight_single_word(Geom:new{ x = 260, y = 70 }) Screen:shot("screenshots/reader_highlight_single_word_pdf_scanned_scroll.png") end) it("should highlight text", function() - highlight_text(readerui, Geom:new{x = 192, y = 186}, Geom:new{x = 280, y = 186}) + highlight_text(Geom:new{x = 192, y = 186}, Geom:new{x = 280, y = 186}) Screen:shot("screenshots/reader_highlight_text_pdf_scanned_scroll.png") end) it("should response on tap gesture", function() - tap_highlight_text(readerui, Geom:new{ x = 260, y = 70 }, Geom:new{ x = 260, y = 150 }, Geom:new{ x = 280, y = 110 }) + tap_highlight_text(Geom:new{ x = 260, y = 70 }, Geom:new{ x = 260, y = 150 }, Geom:new{ x = 250, y = 75 }) Screen:shot("screenshots/reader_tap_highlight_text_pdf_scanned_scroll.png") end) end) describe("for reflowed page", function() before_each(function() UIManager:quit() + + local readerui = ReaderUI:new{ + dimen = Screen:getSize(), + document = DocumentRegistry:openDocument(sample_pdf), + } + readerui:handleEvent(Event:new("SetScrollMode", true)) readerui.document.configurable.text_wrap = 1 + readerui:handleEvent(Event:new("ReflowUpdated")) + UIManager:show(readerui) readerui.paging:onGotoPage(31) + readerui:paintTo(Screen.bb, 0, 0) end) after_each(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 + local readerui = ReaderUI.instance + + if readerui then + readerui.highlight:clear() + readerui.document.configurable.text_wrap = 0 + readerui:handleEvent(Event:new("ReflowUpdated")) + readerui:closeDocument() + readerui:onClose() + end end) it("should highlight single word", function() - highlight_single_word(readerui, Geom:new{ x = 260, y = 70 }) + highlight_single_word(Geom:new{ x = 260, y = 70 }) Screen:shot("screenshots/reader_highlight_single_word_pdf_reflowed_scroll.png") end) it("should highlight text", function() - highlight_text(readerui, Geom:new{ x = 260, y = 70 }, Geom:new{ x = 260, y = 150 }) + highlight_text(Geom:new{ x = 260, y = 70 }, Geom:new{ x = 260, y = 150 }) Screen:shot("screenshots/reader_highlight_text_pdf_reflowed_scroll.png") end) it("should response on tap gesture", function() - tap_highlight_text(readerui, Geom:new{ x = 260, y = 70 }, Geom:new{ x = 260, y = 150 }, Geom:new{ x = 280, y = 110 }) + tap_highlight_text(Geom:new{ x = 260, y = 70 }, Geom:new{ x = 260, y = 150 }, Geom:new{ x = 250, y = 75 }) Screen:shot("screenshots/reader_tap_highlight_text_pdf_reflowed_scroll.png") end) end)