ReaderHighlight: unit test

[hius07]

No idea of the reasons of #11563 (comment) so far.

---

This change is 

[hius07]

Can I trigger the test without merging?
"Test" section contains 289 tests, "Coverage" section (triggered at merging) - 523.

[Frenzie]

If you can't run the tests locally you can toss in a make coverage or something.

koreader/.ci/test.sh

Line 12 in 8530282

make testfront BUSTED_SPEC_FILE="${BUSTED_SPEC_FILE}"

---

"Test" section contains 289 tests, "Coverage" section (triggered at merging) - 523.

This sounds extremely unlikely though. :-) There are some that say #notest. Those might fail in CI for various reasons, so they're only for local. Some of those should be reevaluated now that we're a few years on. Many more say #nocov for similar reasons.

However, the tests are split up over 2 jobs to finish quicker while the coverage does all of them.

[Frenzie]

Concretely:

job 0:

[----------] Global test environment teardown.
[==========] 289 tests from 37 test files ran. (36206.77 ms total)
[  PASSED  ] 289 tests.

job 1:

[----------] Global test environment teardown.
[==========] 250 tests from 36 test files ran. (37772.56 ms total)
[  PASSED  ] 250 tests.

= 289 + 250

[benoit-pierre]

I must say, I don't think I ever managed to run the frontend testsuite successfully on master: I always get some failures in the readerhighlight part (and the occasional segfault).

I have never reported the issue because it works fine on my meson branch, where I've made some changes to speedup the testsuite and I don't have to fiddle with the environment (luarocks, busted, etc…). ¯\(ツ)/¯

2 commits touch spec/unit/readerhighlight_spec.lua: benoit-pierre@c522770 & benoit-pierre@021c1df. I think it's the second one. NOTE: I'm not yet caught up with master.

[Frenzie]

I must say, I don't think I ever managed to run the frontend testsuite successfully on master: I always get some failures in the readerhighlight part (and the occasional segfault).

Can't confirm, unless it somehow changed in the last few months. Normally I only run specific tests locally. (Unless you're referring to Turbo or coverage.)

benoit-pierre@021c1df.

I don't know why that was waiting a whole second but based purely on that being there it would suggest UIManager:nextTick(). On the flip side, the close logic has changed in the intervening years.

[benoit-pierre]

Normally I only run specific tests locally.

Why, too slow?

Anyway, it might fixed be another change to another part of the testsuite, as there's some sort of interaction between the tests.

On master (8530282):

• make testfront:

[  FAILED  ] 3 tests, listed below:
[  FAILED  ] spec/front/unit/document_spec.lua:68: EPUB document module should register droid sans fallback
[  FAILED  ] spec/front/unit/readerhighlight_spec.lua:188: Readerhighlight module highlight for PDF documents in page mode for scanned page without text layer should respond to tap gesture #nocov
[  FAILED  ] spec/front/unit/readerhighlight_spec.lua:218: Readerhighlight module highlight for PDF documents in page mode for reflowed page should response on tap gesture #nocov
[  ERROR   ] 1 error, listed below:
[  ERROR   ] spec/front/unit/readerhighlight_spec.lua:160: Readerhighlight module highlight for PDF documents in page mode for scanned page with text layer should response on tap gesture #nocov

(disregard droid sans fallback which started failing again after installing a new user font)

• make testfront BUSTED_SPEC_FILE=spec/front/unit/readerhighlight_spec.lua: no issue
• make testfront BUSTED_SPEC_FILE='spec/front/unit/readerfooter_spec.lua spec/front/unit/readerhighlight_spec.lua': no issue
• make testfront BUSTED_SPEC_FILE='spec/front/unit/readerbookmark_spec.lua spec/front/unit/readerhighlight_spec.lua':

[  FAILED  ] 3 tests, listed below:
[  FAILED  ] spec/front/unit/readerhighlight_spec.lua:111: Readerhighlight module highlight for EPUB documents should highlight single word
[  FAILED  ] spec/front/unit/readerhighlight_spec.lua:188: Readerhighlight module highlight for PDF documents in page mode for scanned page without text layer should respond to tap gesture #nocov
[  FAILED  ] spec/front/unit/readerhighlight_spec.lua:218: Readerhighlight module highlight for PDF documents in page mode for reflowed page should response on tap gesture #nocov
[  ERROR   ] 1 error, listed below:
[  ERROR   ] spec/front/unit/readerhighlight_spec.lua:160: Readerhighlight module highlight for PDF documents in page mode for scanned page with text layer should response on tap gesture #nocov

[Frenzie]

Why, too slow?

Obviously, but unless you can make all tests run in under a second on a slow laptop I don't see any scenario where that would ever change. (To be clear and for context I made Travis/CircleCI more than 10 minutes faster than it once was.) If I'm working on ReaderHighlight there's no point in wasting time on anything else.

The automated equivalent to that manual action, limiting tests based on changed files the way I did in crengine seems far too risky, but perhaps it could be useful as a development aid.

[benoit-pierre]

Why, too slow?

Obviously, but unless you can make all tests run in under a second on a slow laptop I don't see any scenario where that would ever change.

Locally, make test takes 1m32 on master, and ~14s on my dev branch. Faster on CI too: 29s vs 1m18 (and coverage: 2m32 vs 6m23).

[Frenzie]

That much? Impressive, that's better than I had anticipated. I didn't think there was much more than about 30% to be squeezed out of it. (On CI I mean, of course there are easier gains to be made with more cores.)

Though like I implied, the difference between 10 seconds and 100 seconds is barely perceptible to me. They're both too long, let's go get some tea. ;-)

[benoit-pierre]

It's mostly thanks to parallelization:

• 1 job: 56s (-39%)
• 2 jobs: 30s (-67%)
• 3 jobs: 21s (-77%)

Less resources used on the CI actually, but better used: no parallel runs (example run):

• master:
  
• dev:
  

[benoit-pierre]

On master, I can't run the readerlink tests 2 times without cleaning up the install directory.

Fixed by c7b31e2.

I can now run the full testsuite with #11784. But there are still some rotten things in the busted kingdom. For example, I still get 1 failure when running make all testfront BUSTED_SPEC_FILE='spec/front/unit/readerhighlight_spec.lua spec/front/unit/readerbookmark_spec.lua. I suspect it's because some tests have the bad habit of running some teardown code when starting, cleaning up after a preceding test: so depending on which tests are selected, and their names (since we use --sort-files), the outcome can differ.

[Frenzie]

Yes, some tests may still have issues like that.

[Frenzie]

Fixed by #11784

[hius07]

Thank you.