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

RESOLVED FIXED in Firefox 57



2 years ago
a year ago


(Reporter: jfkthame, Assigned: gbrown)


Firefox 57

Firefox Tracking Flags

(firefox57 fixed)



(1 attachment)



2 years ago
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

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

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

2 years ago
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.
Assignee: nobody → gbrown
Blocks: 1364381
Component: Reftest → Testing
Priority: -- → P1
Product: Testing → Firefox for Android
Version: Version 3 → unspecified


2 years ago
See Also: → bug 742558, bug 961024

Comment 2

2 years ago
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?
Flags: needinfo?(odvarko)
Flags: needinfo?(odvarko) → needinfo?(honzab.moz)
(In reply to Geoff Brown [:gbrown] from comment #2)
 or to change the location of the cache?

Flags: needinfo?(honzab.moz)


2 years ago
See Also: → bug 1389249

Comment 4

2 years ago
(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 6

2 years ago
Created attachment 8896533 [details] [diff] [review]
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. 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?
Attachment #8896533 - Flags: review?(bob)
Attachment #8896533 - Flags: feedback?(jfkthame)

Comment 7

2 years ago
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!
Attachment #8896533 - Flags: feedback?(jfkthame) → feedback+
Comment on attachment 8896533 [details] [diff] [review]
move necko cache location and ensure it is empty

Looks good. Sorry for the delay.
Attachment #8896533 - Flags: review?(bob) → review+

Comment 9

a year ago
Pushed by
Ensure a fresh necko cache for each mochitest and reftest run; r=bc


a year ago
Duplicate of this bug: 1364381

Comment 11

a year ago
Last Resolved: a year ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
You need to log in before you can comment on or make changes to this bug.