Closed
Bug 767449
Opened 12 years ago
Closed 12 years ago
mochitest-robotium doesn't work if /mnt/sdcard is not available
Categories
(Testing :: Mochitest, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: kats, Assigned: kats)
References
Details
Attachments
(2 files, 2 obsolete files)
10.23 KB,
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
10.14 KB,
patch
|
kats
:
review+
|
Details | Diff | Splinter Review |
I have a Tegra board on which the /mnt/sdcard folder is read-only. Turns out a lot of code has /mnt/sdcard hard-coded so it's not easy to switch to some other folder. I wrote a patch to fix this up somewhat. It works in that it's gotten me farther along in trying to run mochitest-robotium on the Tegra board, but it still doesn't run (now it hangs waiting for Gecko:Ready on the testAwesomebar). One caveat with the patch is that ContentProviderTest.java still hard-codes sdcard because I couldn't figure out how to get the instrumentation object in there.
Comment 1•12 years ago
|
||
Comment on attachment 635799 [details] [diff] [review] Pass around the deviceroot instead of hard-coding /mnt/sdcard good method for getting deviceroot in and using it for robotium.config and fennec_ids.txt. We would need to do the same for talos as well.
Comment 2•12 years ago
|
||
If you are not using /mnt/sdcard and /data/local, there are permission issues to deal with. I can't think of many other places where /mnt/sdcard is hardcoded, but I am glad to see us removing that hard requirement (that I put in).
Assignee | ||
Comment 3•12 years ago
|
||
In this case I can use /data/local so everything works as long as it doesn't try to use /mnt/sdcard. (The hang I was seeing earlier turned out to be because the board was stuck on the lock screen).
Assignee | ||
Comment 4•12 years ago
|
||
This fixes it for the ContentProviderTest subclasses as well, and takes out a spurious println I left in. I probably won't get around to running the talos tests soon so please feel free to steal this patch and modify it (or write a follow-up patch) that does the same for talos.
Attachment #635799 -
Attachment is obsolete: true
Attachment #635846 -
Flags: review?(jmaher)
Comment 5•12 years ago
|
||
Comment on attachment 635846 [details] [diff] [review] Pass around the deviceroot instead of hard-coding /mnt/sdcard (v2) Review of attachment 635846 [details] [diff] [review]: ----------------------------------------------------------------- we need to test this on try and see how talos tests do. This could be a bug where we have to land both talos and desktop changes at the same time. ::: mobile/android/base/tests/BaseTest.java.in @@ +51,5 @@ > > @Override > protected void setUp() throws Exception { > + // Load config file from root path (setup by python script) > + String rootPath = FennecInstrumentationTestRunner.getArguments().getString("deviceroot"); should we have error handling here? ::: mobile/android/base/tests/ContentProviderTest.java.in @@ +154,5 @@ > } > > private void loadRobotiumConfig() { > + String rootPath = FennecInstrumentationTestRunner.getArguments().getString("deviceroot"); > + String configFile = FennecNativeDriver.getFile(rootPath + "/robotium.config"); can we get this from FennecNativeDriver?
Attachment #635846 -
Flags: review?(jmaher) → review+
Assignee | ||
Comment 6•12 years ago
|
||
(In reply to Joel Maher (:jmaher) from comment #5) > Comment on attachment 635846 [details] [diff] [review] > > + // Load config file from root path (setup by python script) > > + String rootPath = FennecInstrumentationTestRunner.getArguments().getString("deviceroot"); > > should we have error handling here? > Yeah, probably. I'm not sure what should do if that fails, though. Should we just fall back to /mnt/sdcard? As it is, it will probably throw a NullPointerException shortly after failing to get a rootpath. > > private void loadRobotiumConfig() { > > + String rootPath = FennecInstrumentationTestRunner.getArguments().getString("deviceroot"); > > + String configFile = FennecNativeDriver.getFile(rootPath + "/robotium.config"); > > can we get this from FennecNativeDriver? I don't think so; the ContentProviderTest doesn't ever create a FennecNativeDriver instance. The static call to getFile is the only reference to FennecNativeDriver. I guess we could stash the arguments in FennecNativeDriver instead of FennecInstrumentationTestRunner but I'm not sure if that's any better.
Comment 7•12 years ago
|
||
if we fail to get it, I would like to ensure we have a solid error message printed out to the logfile so we can debug easier (in all cases where we access deviceroot).
Assignee | ||
Comment 9•12 years ago
|
||
Pushed the patch to try: https://tbpl.mozilla.org/?tree=Try&rev=7eea5e516dd3 (In reply to Joel Maher (:jmaher) from comment #7) > if we fail to get it, I would like to ensure we have a solid error message > printed out to the logfile so we can debug easier (in all cases where we > access deviceroot). But we don't have a logfile set up at that point; it happens much later in the test setup. At best I can log it to logcat (and I've updated the patch locally to do that).
Comment 10•12 years ago
|
||
sounds good to me:)
Assignee | ||
Comment 11•12 years ago
|
||
The try run above failed all the talos tests, as expected. This patch for talos should take care of that, but it's completely and utterly untested. I also don't trust my local setup enough to test this patch there, and I can't test it on try. :jmaher, is there someplace this patch can be tested? I think it should work just on it's own and should be landable before the m-c patch. Once this is in and being used on tbpl then we should be able to land the m-c patch.
Attachment #659346 -
Flags: review?(jmaher)
Comment 12•12 years ago
|
||
Comment on attachment 659346 [details] [diff] [review] Talos patch Review of attachment 659346 [details] [diff] [review]: ----------------------------------------------------------------- this looks good. Here is how to test it on try server: 1) take your talos repo+patch, run 'python create_talos_zip.py' 2) that will create a talos.<changesetid>.zip file 3) scp talos.<changesetid>.zip <people_account> 4) in m-c edit /testing/talos/talos.json to point to your http://people.mozilla.org/~name/talos.<changesetid>.zip 5) push to try with try server comment from try chooser: 'try: -b o -p android -u all -t all' 6) profit If that doesn't work for you, I can test it on try.
Attachment #659346 -
Flags: review?(jmaher) → review+
Assignee | ||
Comment 13•12 years ago
|
||
Updated with the error handling as noted in comment #9. I pushed the patches together to try - https://tbpl.mozilla.org/?tree=Try&rev=ad575c64debe - and they were green. Landing the patches separately, however, will cause all of the robo-talos tests to fail. Is there a procedure for landing these simultaneously?
Attachment #635846 -
Attachment is obsolete: true
Attachment #659509 -
Flags: review+
Comment 14•12 years ago
|
||
we need to get it deployed in talos first, a talos.zip file updated to build.mozilla.org, then we can deploy a change to testing/talos/talos.json to use the new talos.zip file at the same time you deploy the change to robocop.
Assignee | ||
Comment 15•12 years ago
|
||
Sounds good. I landed the talos patch: https://hg.mozilla.org/build/talos/rev/bab61a672f78
Comment 16•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2c1a32f92d28
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•