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] Stop the kobo_touch_probe test from causing problems for other tests. #3877

Merged
merged 1 commit into from Apr 14, 2018

Conversation

Projects
None yet
2 participants
@edorfaus
Contributor

edorfaus commented Apr 14, 2018

Previously, it caused problems because it was overriding G_reader_settings,
which caused the translator test to fail on the second (but not first) run.


tl;dr: this is an example of one test affecting another negatively, and I tried other ways to fix it too, but at least this way seems to work. I tried to explain the underlying problem and my reasoning below.

This one took me a little while, not just to isolate where and figure out what was going on, but also to fix, as there's apparently some magic in there that was working against me. (It doesn't really help that I'm pretty much learning Lua as I go along here, as well as the Koreader codebase...)

The whole thing started with me being annoyed by and wanting to fix a failure I was seeing in the translator test, figuring that the test probably wasn't cleaning up after itself or something like that (since it didn't fail on the first run, only from the second). That wasn't actually the case though - or rather, only sort of, and not that test's fault. It is actually removing again the setting it adds. However, the touch-probe test caused it to use the wrong settings file, as the test harness uses a different file than the tool does, and although the harness removes that settings file, it doesn't remove the .old file, so on the second run the probe test ended up loading back in the previous settings, from before the translator's setting was removed, while the test assumes default values are in use.

I first tried fixing this in the probe test itself, by saving G_reader_settings before requireing the file, and restoring it after - but I couldn't get it to work that way. I tried both with a teardown function and in the test function itself, and stdout debugging in those showed that it was getting set back to the original value - but by the time the next test file got loaded/run, it was set to the overridden value again. My best guess is that require allows the newly loaded file to change some things (namely globals) that its caller (the test) for some reason can't.

[fix] Stop the kobo_touch_probe test from causing problems for other …
…tests.

Previously, it caused problems because it was overriding G_reader_settings,
which caused the Translator test to fail on the second (but not first) run.
@Frenzie

This comment has been minimized.

Member

Frenzie commented Apr 14, 2018

There's at least one test around here with the opposite problem: it breaks in isolation (or if not in the "right" order). See 5bd288e. I haven't found the time to look at it, but it should be fixed sometime.

I wrote that language spec, but I don't think I ever looked at the touch probe spec. Thanks for finding a solution. :-)

@Frenzie Frenzie merged commit 58d9f5c into koreader:master Apr 14, 2018

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

@Frenzie Frenzie added the bug label Apr 14, 2018

@edorfaus edorfaus deleted the edorfaus:bugfix/translator-test-failure branch Apr 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment