1388883 - Running reftests on an Android device may use stale cached test/reference files

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Testing, defect, P1)

Description

[Jonathan Kew [:jfkthame]]

STR (for an Android device, using locally-built Fennec from a mozilla-central checkout on macOS):

- Do "./mach reftest path/to/test/directory" to run a bunch of tests

- Edit one of the test or reference files, so as to fix a failure (or introduce a new one, for testing purposes)

- Do "./mach reftest path/to/test/directory" again

Expected:
The second test run should use the modified files and hence give different results than the first.

Actual:
The second test run uses the old, unmodified files.

Rebuilding/repackaging/reinstalling Fennec from the desktop system does not help; the stale test files are still used on the device.

To avoid the problem, it is necessary to "Clear Cache" on the Android device. This is a pain when attempting to develop/fix tests -- both because it adds a cumbersome step to the edit/test cycle, and because it's easy to forget to do it and thus waste time testing something other than what is intended -- and potentially much more time trying to understand results that don't match the current test files.

In local testing, I found that hacking netwerk/test/httpserver/httpd.js to forcibly add a "Cache-Control: must-revalidate" header to all its responses appears to work around the issue, though I don't know what additional implications this might have for other parts of our testing.

Comment 1

[Geoff Brown [:gbrown]]

Wow, this is interesting! It sounds like the cached copy is coming from the necko cache. That cache was, at least in the distant past, in the firefox profile, which is deleted between test runs. Has the necko cache moved to the Android cache now? Does it survive there even after Fennec is re-installed? I think that would explain everything described here. I will try to reproduce and verify.

Comment 2

[Geoff Brown [:gbrown]]

Thanks to the STR, this was easy to verify. In my experience - running 'mach reftest layout/reftests/inline' with/without modification - a cached file was used most of the time, but not always. (I suppose the modified test file might be evicted, or expire from the necko cache.) If I cleared the Android cache for Fennec before a run - from Android Settings, or with 'adb shell pm clear org.mozilla.fennec_...' - test results were always consistent with the test file on the host.

On the Android 4.3 emulator, after a test run, I see the necko cache at /data/data/org.mozilla.fennec_gbrown/cache/profile/cache2. That seems consistent with intended design - see See Also bugs. 

AFAIK, our test harnesses do nothing to manipulate this cache. (Test harnesses set up a fresh profile for each run, but the cache is independent of the profile, on Android.)

The test harnesses could easily run 'adb shell pm clear <app>' before starting tests, and that would work in some cases, but I believe it would fail without root privileges.

I think we should not modify httpd.js or otherwise modify Cache-Control or related headers. That might interfere with some test results and performance.

:honza -- Do you have any ideas here? Are there prefs or environment variables that could be used to signal necko to discard any existing cache found on startup, or to change the location of the cache?

Comment 3

[Honza Bambas (:mayhemer)]

(In reply to Geoff Brown [:gbrown] from comment #2)
 or to change the location of the cache?

browser.cache.disk.parent_directory

Comment 4

[Geoff Brown [:gbrown]]

(In reply to Honza Bambas (:mayhemer) from comment #3)
> (In reply to Geoff Brown [:gbrown] from comment #2)
>  or to change the location of the cache?
> 
> browser.cache.disk.parent_directory

That works great - thanks :mayhemer (sorry for the confusion :Honza!).

Comment 5

[Geoff Brown [:gbrown]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4b0bc2ea6d30c652d5bbddc316ae3942b422eb1f

Comment 6

[Geoff Brown [:gbrown]]

Attachment: move necko cache location and ensure it is empty

In each of the Android mochitest and reftest harnesses, I've used a pref to change the location of the necko cache from its default to a new location within the "test root". I explicitly delete that directory at harness cleanup and again at initialization time (in case the previous run was interrupted).

Testing locally ('mach reftest ...' with emulator on Ubuntu 16), I now find the test results are always consistent with my changes to the test files on the host.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4b0bc2ea6d30c652d5bbddc316ae3942b422eb1f confirms I've done no obvious harm to emulator unit tests.

I had been concerned that deleting the cache after a long test run might make a test job run for too long, but spot checks on the try push suggest there's no obvious difference to job run time.


Jonathan - Does this patch resolve the issue for you?

Comment 7

[Jonathan Kew [:jfkthame]]

Comment on attachment 8896533 [details] [diff] [review]
move necko cache location and ensure it is empty

Review of attachment 8896533 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, this appears to work as expected for me. Thanks!

Comment 8

[Bob Clary [:bc] (inactive)]

Comment on attachment 8896533 [details] [diff] [review]
move necko cache location and ensure it is empty

Looks good. Sorry for the delay.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/40095d291eb4
Ensure a fresh necko cache for each mochitest and reftest run; r=bc

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/40095d291eb4