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)

All
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: kats, Assigned: kats)

References

Details

Attachments

(2 files, 2 obsolete files)

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 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.
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).
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).
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 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+
(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.
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).
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).
sounds good to me:)
Attached patch Talos patchSplinter Review
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 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+
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+
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.
Depends on: 790271
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.

Attachment

General

Created:
Updated:
Size: