This is a bug to track getting robotium enabled in birch builds. Patch upcoming. This patch probably needs an update to the build platform to install the robotium jar into the java classpath so that it will compile. But I think the makefile pieces are properly there. Trevor, do we really need 3 different png icons for it? Or can we condense those to one? This patch takes the code in Trevor's github repo for Robocop and puts it into birch properly with a proper Makefile.in for our universe.

(In reply to Clint Talbert ( :ctalbert ) from comment #0) > Trevor, do we really need 3 different png icons for it? Or can we condense > those to one? No, there's probably still a lot of java excess I can clean off once we get something working.

This patch works on my system, although fennec crashed so I couldn't test it. Here are some hardcoded items: build/mobile/robocop/Makefile.in: +JAVA_CLASSPATH = $(ANDROID_SDK)/android.jar -classpath /home/joel/mozilla/android/robotium-solo-2.5.jar + $(AAPT) package -f -M $(srcdir)/AndroidManifest.xml -I $(ANDROID_SDK)/android.jar -I /home/joel/mozilla/android/robotium-solo-2.5.jar -S res -F $@ + jarsigner -keystore ~/.android/debug.keystore -storepass android -keypass android $@ androiddebugkey toolkit/mozapps/installer/packager.mk: + jarsigner -keystore ~/.android/debug.keystore -storepass android -keypass android fennec_temp.apk androiddebugkey && \ this boils down to 2 things to fix: 1) getting the path to robotium-solo-2.5.jar into the build system 2) using jarsigner (I can't figure out how this is used for regular android)

with this patch we have: $(objdir)/build/mobile/robocop/robocop.apk then: $(objdir): make package $(objdir)/dist/fennec-10.0a1.en-US.android-arm.apk $(objdir)/dist/fennec-robocop.apk (fixed up with https://wiki.mozilla.org/QA/Automation_Services/Projects/FennecAutomation/FAQ) then: $(objdir): make package-tests $(objdir)/dist/test-package-stage/bin/robocop.apk after installing fennec-robocop.apk and robocop.apk, you can run the tests by doing: adb shell am instrument -w org.mozilla.roboexample.test/android.test.InstrumentationTestRunner

Added updated xpi and java files. Don't know how the xpi will fit into the make process yet.

If you attach the source files for the .xpi, we can build that and in the scenario of android we can setup the profile to include this restartless extension.

Added patch to fix some points in the build process, and fixes install of robotium.jar Still need to manually edit the make file to point to your robotium.jar

Actually fixes the problem that the second one wanted.

updated and a much cleaner implementation. You now need to add to your .mozconfig: # for testing the nativeUI ac_add_options --with-robotium-path="$HOME/mozilla/android/robotium-solo-2.5.jar" Also there is a bug where we don't delete or overwrite fennec-robocop.apk and if it exists we fail to compile.

There was a slight mixup with the awesomebar and awesomebar_text, and a change to the GeckoLayout item. This makes it work, but there are still OOM errors popping up.

latest bits which does a cleaned up pan test, reads a /mnt/sdcard/robotium.config to find the profile, reads a /mnt/sdcard/fennec_ids.txt to find the mapping of all the fennec ids defined in embedded/android/R.java. Right now there is a script testing/mochitest/tpan.py which will copy fennec_ids.txt and robotium.config to /mnt/sdcard. Should have the script updated so we can have 'python runtestsremote.py --robotium' where it will handle the setup and robotium work.

Now to run: make -f client.mk cd objdir make package make package-tests export TEST_DEVICE = 1.2.3.4 export MOZ_HOST_BIN=~/objdir/dist/bin make mochitest-robotium Issue's I'm still looking at: -I wanted to load right into the web page. -Dragging is currently dependant on a decent resolution.

updated patch, works well with 'make mochitest-robotium' as well as running from a tests package. Plumbing works with sutagent, but sutagent crashes during a run. Otherwise this is a cleaned up patch getting closer to a reviewable state.

Splits up the tests into multiples. Doesn't work currently because of between-test failures with gecko.

Fixed last patch.

Still cleaning up that rushed test-splitting patch.

I have created a WebDriver-esque API to drive Fennec. It is split in to 2 different parts. The "driver" which finds stuff for you and the "element" that you interact with as well as a working test. You can see the details at https://gist.github.com/1415531. My approach removes the dependency on JUnit for the driving part and we just have it in the tests. This makes it cleaner as an automation framework If people are happy with this then I will attach patches.

To go on the 0.95 patch.

Round 2. Still need to work on the hg patch making.

patch with all the makefile hooks, robocop building and testapi stuff integrated. verified it would build and install on my system. TODO: * cleanup bootstrap.js to make it cleaner * resolve and pending issues with testAPI * ensure we have 5 tests written and they can run reliably * documentation of tool and how to write tests * find a location for the test files (shouldn't be in mobile/build/robocop) * consider fixing the .in preprocessor to put in the objdir instead

Comment on attachment 578481 [details] [diff] [review] updated patch (0.97) FennecNativeElement.java.in, FennecNativeDrive.java.in, Element.java.in, Driver.java.in should probably have licences added to them.

Updated with testApi, working extension, manifests, 4 test cases, and a lot of cleanup. TODO: * cleanup bootstrap.js to make it cleaner (just the roboquit stuff) * Make the example test cases actually do something! * documentation of tool and how to write tests * find a location for the test files (shouldn't be in mobile/build/robocop) * consider fixing the .in preprocessor to put in the objdir instead

oh, and we need to make it do logging as we would for regular mochitest. I think if a couple people can find this patch usable and not offensive we can start getting other to write tests while we work on the integration bits.

oops, wrong patch was uploaded, this is everything! here is how I run the test from a objdir/dist/test-package-stage/mochitest dir: python runtestsremote.py --robocop=../bin --xre-path=../../../../../inbound/obj-i686-pc-linux-gnu/dist/bin --deviceIP 1.2.3.4 --dm_trans=adb --app=org.mozilla.fennec_jmaher

Comment on attachment 579439 [details] [diff] [review] integrate robotium into build system (0.99) Missing tests folder.

updated to fix the parseid problem as well as adding in the tests folder and the license blocks.

Comment on attachment 579567 [details] [diff] [review] integrate robotium into build system (0.99) Review of attachment 579567 [details] [diff] [review]: ----------------------------------------------------------------- This looks great - I don't have any serious concerns. Just a few questions about bits that look a little rough... ::: build/mobile/robocop/FennecNativeDriver.java.in @@ +162,5 @@ > + height = jo.getInt("height"); > + pageHeight = jo.getInt("y"); > + > + } catch( Throwable e) { > + Log.i("Testing", "ScrollReceived, but read wrong!"); Maybe Log.i("Robocop"... would be better? (There are a few "Testing" log tags, and some "Robocop"'s elsewhere.) ::: build/mobile/robocop/FennecNativeElement.java.in @@ +162,5 @@ > + } > + > + @Override > + public void sendKeys(String input) { > + try { Thread.sleep(2000); } Are these sleep calls necessary? Are we just pausing so that steps are visible / distinct? ::: testing/mochitest/runtestsremote.py @@ +384,5 @@ > + cmd.append("org.mozilla.roboexample.test/android.test.InstrumentationTestRunner") > + retVal = dm.checkCmd(cmd) > + else: > + # SUTAgent needs to install robocop and not crash when we launch robocop. > + retVal = dm.launchProcess(["am", "instrument", "-w", "org.mozilla.roboexample.test/android.test.InstrumentationTestRunner"]) It would be nice to launch the same way for adb and sut. I am trying to unify the adb and sut launchProcess()'s in bug 705192...but that's not ready yet. Even so, why is there no "install -r" and no "-e class" for sut?

updated with cleaned up and new tests. the test files now live in $(topsrcdir)/mobile/android/base/tests. Builds on a clean system just fine. TODO: * documentation of tool and how to write tests (wiki page) * remove hack to download planet.html in runtestsremote.py, add test files and pages to mobile/android/base/tests folder * resolve fennec-robocop.apk (this won't work for releng) * resolve running via SUT agent. * resolve assert statements to work with mochitest style logging. * fix quirky makefile.in that lives in build/mobile/robocop, but lists files in the tests folder

Per f2f discussion with tfair, it will be really helpful to add support for packaging tests in a specific directory. (instead of having the individual files listed in Makefile.in)

We might not be able to make this work so smoothly, but we will try to make it simpler. For what :gkw is looking to do, we think it could work for him to generate a set of .java files and put them in the build tree (mobile/android/base/test/), rebuild and run the tests. Some issues to resolve: * removing the files hardcoded in the makefile.in * allowing for specific files to be run via the runtestsremote.py (just run the new files only) * figuring out how all this works with the robocop.ini

I stumbled on a minor bug: FennecNativeElement.sendKeys() does not handle upper case letters correctly. Suspect code is: + else if( c >= 'A' && c <='Z') { + instr.sendCharacterSync(KeyEvent.KEYCODE_A+(int)(c-'a')); instr.sendStringSync(""+c) works fine.

this is just the tests only part of the patch. I want to get these landed and in so we can start making smaller patches faster. Please take a few minutes to review these tests for style, basic functionality and usability. We can either fix these tests or file followup bugs.

this is the core robocop.apk toolchain. This isn't the glue to hook it up and it references the tests in the previous patch I uploaded. There will be 2 more patches: * mochitest harness integration * build system integration Please review this patch for style, core logic and usability. I want to start landing this code so we can churn quicker on turning these tests on and writing new tests.

remaining files required to get robotium up and running.

Comment on attachment 580705 [details] [diff] [review] core robocop.apk toolchain (1.0) Review of attachment 580705 [details] [diff] [review]: ----------------------------------------------------------------- You have both the *.*.in and the *.* files in this patch. Please clean up. Also, the product of the pre-processing must go in the objdir, not the srcdir ::: build/mobile/robocop/Makefile.in @@ +69,5 @@ > + res/layout/main.xml \ > + res/values/strings.xml \ > + $(NULL) > + > +GARBAGE += \ GARBAGE needs to include all the products of your preprocessing @@ +89,5 @@ > +# Override rules.mk java flags with the android specific ones > +include $(topsrcdir)/config/android-common.mk > + > +$(_JAVA_HARNESS): % : %.in > + $(PYTHON) $(topsrcdir)/config/Preprocessor.py $(DEFINES) $< > $(srcdir)/$@ put the results in the objdir $(PYTHON) $(topsrcdir)/config/Preprocessor.py $(DEFINES) $< > $@ @@ +91,5 @@ > + > +$(_JAVA_HARNESS): % : %.in > + $(PYTHON) $(topsrcdir)/config/Preprocessor.py $(DEFINES) $< > $(srcdir)/$@ > + > +$(srcdir)/AndroidManifest.xml: % : %.in this needs to go in the objdir

Comment on attachment 580704 [details] [diff] [review] initial set of robocop tests (1.0) Overall not bad. * Remove the *.java and leave the *.java.in files * Heed Brad's words about where to put the post-processed files. * We might want to evolve some of this code into utils, like setting up the profile. * Be careful with driver.findElement since it depends on Android inflating the View so the element exists. You might want to include a timeout/wait loop in there like you have for driver.waitForGeckoEvent r- for the post-processing. the other nits can be addressed later imo

(In reply to Mark Finkle (:mfinkle) from comment #37) > * Be careful with driver.findElement since it depends on Android inflating > the View so the element exists. You might want to include a timeout/wait > loop in there like you have for driver.waitForGeckoEvent So, with findElement, there is no accessing of the Fennec elements, and is only robocop-internal, so that won't hang. When getText, or other element-specific accessing methods are called, the "findElementByID" seems to be a simple pointer access, and returns null if it can't find it, so i don't have too much worry around timing out, although it could always be added.

Comment on attachment 580705 [details] [diff] [review] core robocop.apk toolchain (1.0) Review of attachment 580705 [details] [diff] [review]: ----------------------------------------------------------------- r- for the upper case and : key handling problems -- other issues are minor. ::: build/mobile/robocop/FennecNativeDriver.java.in @@ +93,5 @@ > + this.activity = activity; > + this.solo = robocop; > + > + // Set up table of fennec_ids. > + locators = convertTextToTable(getFile("/mnt/sdcard/fennec_ids.txt")); There is an issue with this path if using adb devicemanager *and* /data/local/tests exists: In that case, dm_adb will check to see if it can use its run-as / cp trick to copy files to /data/local/tests, that will probably succeed, and it will then use run-as in pushFile calls. But when pushFile is used on /mnt/sdcard, the run-as commands will likely fail (fennec probably does not have permission to write to /sdcard). The problem arises because we are pushing fennec_ids.txt, etc to a directory outside of device root. One way of solving this is to duplicate the python getDeviceRoot logic in the driver and then access <deviceroot>/fennec_ids.txt. Alternately, we can look at this as a devicemanager weakness and update pushFile to not use run-as if the destination is outside of deviceRoot. Or we can just warn people to delete /data/local/tests! ::: build/mobile/robocop/FennecNativeElement.java.in @@ +105,5 @@ > + if(vg.getChildAt(i) instanceof TextView) { > + text = ((TextView)vg.getChildAt(i)).getText(); > + } > + } //end of for > + } //end of else if Handle the else case too: at least log it. @@ +114,5 @@ > + try { Thread.sleep(1000); } > + catch (InterruptedException e) { > + e.printStackTrace(); > + } > + return text.toString(); On a couple of test runs, this line threw a null pointer exception. I have not determined root cause, but perhaps it would be best to check for and handle text==null here. @@ +150,5 @@ > + } > + > + @Override > + public void sendKeys(String input) { > + try { Thread.sleep(1000); } Surely there is no need to pause *before* sending key events -- this sleep seems extraneous. @@ +161,5 @@ > + instr.sendCharacterSync(KeyEvent.KEYCODE_A+(int)(c-'a')); > + continue; > + } > + else if( c >= 'A' && c <='Z') { > + instr.sendCharacterSync(KeyEvent.KEYCODE_A+(int)(c-'a')); See comment 32. @@ +193,5 @@ > + //Make sure that the Shift is recognized first. > + try{ Thread.sleep(500);} catch (InterruptedException e) { > + e.printStackTrace(); > + } > + instr.sendCharacterSync(KeyEvent.KEYCODE_SEMICOLON); On my Galaxy Tab, this comes out as ; rather than the intended :. Can sendStringSync(":") be used instead?

(In reply to Geoff Brown [:gbrown] from comment #39) > @@ +150,5 @@ > > + } > > + > > + @Override > > + public void sendKeys(String input) { > > + try { Thread.sleep(1000); } > > Surely there is no need to pause *before* sending key events -- this sleep > seems extraneous. This sleep is added because in the test framework there isn't anything which identifies that the java UI is fully loaded. There were cases where the first few characters were being forgotten because the test handler tried submitting characters before the URL bar was fully loaded.

Comment on attachment 580704 [details] [diff] [review] initial set of robocop tests (1.0) Review of attachment 580704 [details] [diff] [review]: ----------------------------------------------------------------- testAwesomebar.java has + for(int i = 0; i < 10; i++) { + solo.drag(midX,midX,midY,endY,10); + try { + Thread.sleep(200); + } catch (InterruptedException e) { + e.printStackTrace(); + } + } I would create a method on Element called drag and copy the code in there. In a number of files you have + urlbar.clickSpecialKey(Element.SpecialKey.ENTER); + driver.waitForGeckoEvent("DOMContentLoaded"); wouldn't it be worth having waitForGeckoEvent on Element or in a utils class that both Driver and Element can access? * I am also concerned that we are not using a definitive assert framework. It is the test runners job to do the assert not the browser automation framework * The initial implementation was based on WebDriver API, which is the basis for our web testing and Marrionette, and we can't have asserts on that API so we should make Robocop similar

Fixed up both test patch and harness patch with following updates: -Only .in files -Preprocessed files go in objdir -Properly cleaning objdir -sendKeys() simplified to wrap Instrumentation sendStringSync() -getText cleaned to throw RoboCopExceptions with missing text -sendKeys(), sendSpecialCharacter(), drag(), and waitForGeckoEvent() moved to Actions class to differentiate from element-based actions.

(See tests patch comment update above)

Comment on attachment 581503 [details] [diff] [review] initial set of robocop tests (1.01) >+ @Override >+ protected void setUp() throws Exception >+ { >+ // Load config file from sdcard (setup by python script) >+ String configFile = FennecNativeDriver.getFile("/mnt/sdcard/robotium.config"); >+ HashMap config = FennecNativeDriver.convertTextToTable(configFile); >+ >+ // Create the intent to be used with all the important arguments. >+ Intent i = new Intent(Intent.ACTION_MAIN); >+ String argsList = "-no-remote -profile " + (String)config.get("profile"); >+ i.putExtra("args", argsList); >+ >+ //Start the activity >+ setActivityIntent(i); >+ activity = getActivity(); >+ >+ //Set up Robotium.solo and Driver objects >+ solo = new Solo(getInstrumentation(), getActivity()); >+ driver = new FennecNativeDriver(activity, solo); >+ actions = new FennecNativeActions(activity, solo, getInstrumentation()); >+ driver.setLogFile((String)config.get("logfile")); >+ } I'd love to get this somewhere we don't need to repeat it for each test. Otherwise, I think this looks good. I'm sure I'll cry about things once I start writing tests myself, but this looks pretty clean and straight forward. Great work you guys

Comment on attachment 581504 [details] [diff] [review] core robocop.apk toolchain (1.01) Review of attachment 581504 [details] [diff] [review]: ----------------------------------------------------------------- Looks good!

Comment on attachment 581504 [details] [diff] [review] core robocop.apk toolchain (1.01) Review of attachment 581504 [details] [diff] [review]: ----------------------------------------------------------------- ::: build/mobile/robocop/Makefile.in @@ +56,5 @@ > + RoboCopException.java \ > + FennecNativeDriver.java \ > + FennecNativeActions.java \ > + > +_JAVA_TESTS := $(patsubst %.in,%,$(wildcard $(TESTPATH)/*.java.in)) I recall needing to do something like this: _JAVA_TESTS := $(patsubst $(TESTPATH)/%.in,%,$(wildcard $(TESTPATH)/*.java.in)) this results in a list of filename.java without the path or .in, then we can use the filenames below. @@ +103,5 @@ > + $(PYTHON) $(topsrcdir)/config/Preprocessor.py $(DEFINES) $< > $@ > + > +$(_JAVA_TESTS): % : %.in > + $(NSINSTALL) -D $(TESTPATH) > + $(PYTHON) $(topsrcdir)/config/Preprocessor.py $(DEFINES) $< > $@ Then I modified this to do: $(NSINSTALL) -D $(DEPTH)/mobile/android/base/tests $(PYTHON) $(topsrcdir)/config/Preprocessor.py $(DEFINES) (TESTPATH)/$< > $(DEPTH)/mobile/android/base/tests/$@ @@ +116,5 @@ > +classes.dex: $(_JAVA_HARNESS) > +classes.dex: $(_JAVA_TESTS) > +classes.dex: $(TEST_FILES) > + $(NSINSTALL) -D classes > + $(JAVAC) $(JAVAC_FLAGS) -d classes $(srcdir)/$(JAVAFILES) $(_JAVA_HARNESS) $(_JAVA_TESTS) and finally: $(JAVAC) $(JAVAC_FLAGS) -d classes $(srcdir)/$(JAVAFILES) $(_JAVA_HARNESS) $(addprefix $(DEPTH)/mobile/android/base/tests/,$(_JAVA_TESTS))

Comment on attachment 581504 [details] [diff] [review] core robocop.apk toolchain (1.01) Review of attachment 581504 [details] [diff] [review]: ----------------------------------------------------------------- ::: build/mobile/robocop/FennecNativeActions.java.in @@ +112,5 @@ > + e.printStackTrace(); > + } > + } > + > + //NOTE: this currently causes a nullpointer exception, we need to return a GeckoApp object I assume you actually want to return an App object (subclass of GeckoApp). That may be the source of your problem @@ +131,5 @@ > + finalParams[0] = geckoEvent; > + finalParams[1] = Proxy.newProxyInstance(classLoader, interfaces, new wakeInvocationHandler()); > + registerGEL.invoke(null, finalParams); > + asleep = true; > + while(asleep) { Let's use something better than a busy loop. A SynchronousQueue perhaps? @@ +165,5 @@ > + instr.sendCharacterSync(KeyEvent.KEYCODE_ENTER); > + break; > + default: > + } > + try { Thread.sleep(500); } what is this sleep for? @@ +173,5 @@ > + } > + > + @Override > + public void sendKeys(String input) { > + try { Thread.sleep(1000); } what is this sleep for? @@ +178,5 @@ > + catch (InterruptedException e) { > + e.printStackTrace(); > + } > + instr.sendStringSync(input); > + try { Thread.sleep(500); } what is this sleep for? ::: build/mobile/robocop/FennecNativeDriver.java.in @@ +111,5 @@ > + Class gfx = classLoader.loadClass("org.mozilla.gecko.gfx.PanningPerfAPI"); > + _startFrameRecording = gfx.getDeclaredMethod("startFrameTimeRecording"); > + _stopFrameRecording = gfx.getDeclaredMethod("stopFrameTimeRecording"); > + } catch (ClassNotFoundException e) { > + e.printStackTrace(); in some places you use Log.e() and others you use e.printStackTrace(), choose one and be consistent ::: build/mobile/robocop/FennecNativeElement.java.in @@ +85,5 @@ > + private Activity elementActivity; > + > + @Override > + public String getText() { > + try { Thread.sleep(1000); } what is this sleep for? @@ +117,5 @@ > + } // end of run() method definition > + } // end of anonymous Runnable object instantiation > + ); > + //Wait for the UiThread code to finish running > + try { Thread.sleep(1000); } what is this sleep for? ::: build/mobile/robocop/R.java @@ +1,5 @@ > +/* AUTO-GENERATED FILE. DO NOT MODIFY. > + * > + * This class was automatically generated by the > + * aapt tool from the resource data it found. It > + * should not be modified by hand. just generate it at build time ::: build/mobile/robocop/res/layout/main.xml @@ +6,5 @@ > + > + <TextView > + android:layout_width="fill_parent" > + android:layout_height="wrap_content" > + android:text="@string/hello" /> probably don't need the hello string ::: build/mobile/robocop/res/values/strings.xml @@ +1,4 @@ > +<?xml version="1.0" encoding="utf-8"?> > +<resources> > + > + <string name="hello">Hello World!</string> again, unneeded

Comment on attachment 581503 [details] [diff] [review] initial set of robocop tests (1.01) Review of attachment 581503 [details] [diff] [review]: ----------------------------------------------------------------- This looks good to me

Comment on attachment 581504 [details] [diff] [review] core robocop.apk toolchain (1.01) Review of attachment 581504 [details] [diff] [review]: ----------------------------------------------------------------- ::: build/mobile/robocop/AndroidManifest.xml.in @@ +11,5 @@ > + android:name="android.test.InstrumentationTestRunner" > + android:targetPackage="@ANDROID_PACKAGE_NAME@" /> > + > + <application > + android:icon="@drawable/ic_launcher" Oops - the ic_launcher.png files were dropped from this patch, but are referenced and needed for a clean build!

Fixed: -Removed a bunch of sleeps and commented the remainder. -Replaced busy loop with SynchronousQueue -Fixed Invocation Handling exception. -Removed res files and R.java -Changed all log.e to e.printStackTrace()

Comment on attachment 582325 [details] [diff] [review] core robocop.apk toolchain (1.02) gbrown: Fixed the resource issue with latest patch.

(Fixed build issue)

landed the tests on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/709945fc4fd4 NOTE: there is a followup bug to the tests coming shortly and there is the harness and integration bugs as well.

Comment on attachment 582340 [details] [diff] [review] core robocop.apk toolchain (1.02) Review of attachment 582340 [details] [diff] [review]: ----------------------------------------------------------------- ::: build/mobile/robocop/FennecNativeElement.java.in @@ +78,5 @@ > + View awesomebar = (View)currentActivity.findViewById(id); > + awesomebar.performClick(); > + } > + }); > + try { Thread.sleep(500); } what is this sleep for? @@ +89,5 @@ > + private Activity elementActivity; > + > + @Override > + public String getText() { > + try { Thread.sleep(1000); } what is this sleep for? @@ +121,5 @@ > + } // end of run() method definition > + } // end of anonymous Runnable object instantiation > + ); > + //Wait for the UiThread code to finish running > + try { Thread.sleep(1000); } actually wait for it then, don't sleep

Comment on attachment 582340 [details] [diff] [review] core robocop.apk toolchain (1.02) Review of attachment 582340 [details] [diff] [review]: ----------------------------------------------------------------- ::: build/mobile/robocop/FennecNativeActions.java.in @@ +114,5 @@ > + e.printStackTrace(); > + } > + } > + > + //NOTE: this currently causes a nullpointer exception, we need to return a GeckoApp object what is the stack from this exception? This should be fixed before landing @@ +151,5 @@ > + finalParams[1] = proxy; > + registerGEL.invoke(null, finalParams); > + > + waitqueue.poll(); > +//NOTE: this is commented out to avoid a nullpointer exception until we fix wakeInvocationHandler. what is commented out? ::: build/mobile/robocop/Makefile.in @@ +63,5 @@ > + $(TESTPATH)/robocop.ini \ > + parse_ids.py \ > + $(NULL) > + > +#RES_FILES = \ don't land a commented out line @@ +86,5 @@ > +DEFINES += \ > + -DANDROID_PACKAGE_NAME=$(ANDROID_PACKAGE_NAME) \ > + $(NULL) > + > +#GARBAGE_DIRS += res don't land commented out

Removed unnecessary comments, and replaced sleeps with synchronization.

Comment on attachment 582416 [details] [diff] [review] core robocop.apk toolchain(1.02.1) Review of attachment 582416 [details] [diff] [review]: ----------------------------------------------------------------- It's great to see all those sleep() calls gone...but now the tests don't run: the key events don't seem to be delivered to the awesome bar. Does anyone else have this problem? ::: build/mobile/robocop/AndroidManifest.xml.in @@ +11,5 @@ > + android:name="android.test.InstrumentationTestRunner" > + android:targetPackage="@ANDROID_PACKAGE_NAME@" /> > + > + <application > + android:icon="@drawable/ic_launcher" Delete this line -- it causes a build failure.

Comment on attachment 582416 [details] [diff] [review] core robocop.apk toolchain(1.02.1) Review of attachment 582416 [details] [diff] [review]: ----------------------------------------------------------------- code-wise this looks good. I'll let you and Geoff figure out what is needed to make this work from here. I assume you need to add some synchronization in the two place you had sleeps in the last patch and didn't add synchronization here.

Adding back a sleep before sending keys helps immensely. I am trying to use Instrumentation.waitForIdleSync and/or waitForGeckoEvent calls to do this properly...but realized tonight that waitForGeckoEvent is broken -- should be able to sort it out tomorrow.

More patches still to land...

The important change here is to wait for the "Gecko:Ready" event before starting each test. Without this change, I often see tests fail because the uri is not received in the awesome bar. Strategic use of Instrumentation.waitForIdleSync also seems to improve reliability. Order of arguments to driver.is switched to provide correct "expected" and "got" in error messages.

Comment on attachment 582416 [details] [diff] [review] core robocop.apk toolchain(1.02.1) Review of attachment 582416 [details] [diff] [review]: ----------------------------------------------------------------- In waitForGeckoEvent, the poll() call does not actually wait; this should probably be take(), as used elsewhere. My problem with key events not reaching the awesome bar is probably better addressed in the tests themselves -- I posted a separate patch for that. r+ subject to removing the ic_launcher and the problem with waitForGeckoEvent. Since tfairey is away, I'll update the patch myself.

Minor changes to tfair's patch, as noted in review: - remove ic_launcher from AndroidManifest.xml.in - fix waitForGeckoEvent - improved error reporting

Comment on attachment 583303 [details] [diff] [review] core robocop.apk toolchain (1.03) Carrying forward r+, r=blassey

Comment on attachment 583325 [details] [diff] [review] minor changes to initial set of robocop tests to improve reliability Review of attachment 583325 [details] [diff] [review]: ----------------------------------------------------------------- looks great and much cleaner.

inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/65f3838752d8

some small tweaks included in here to use a smaller page. inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/b84f3d113058

updated to use the robotium-solo-3.0.jar from the source tree. Please look at the jarsigner stuff and ensure that is valid. Lets get the ball rolling on this patch.

Comment on attachment 583774 [details] [diff] [review] integrate robotium into build system (1.1) Review of attachment 583774 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/mozapps/installer/packager.mk @@ +329,5 @@ > GECKO_APP_AP_PATH = $(call core_abspath,$(DEPTH)/mobile/android/base) > endif > > +ROBOCOP = $(NULL) > +ifdef ENABLE_TESTS nit, use an else @@ +331,5 @@ > > +ROBOCOP = $(NULL) > +ifdef ENABLE_TESTS > +ROBOCOP = \ > + cp $(PACKAGE) fennec.zip && \ nit, let's call this robocop.zip to avoid confusion

Oops -- better reverse the order of these lines: + os.system("mv mpl-tri-license-c tests/robocop/robocop.html") + os.system("mkdir tests/robocop")

https://hg.mozilla.org/mozilla-central/rev/65f3838752d8 https://hg.mozilla.org/mozilla-central/rev/b84f3d113058

1.43 + driver.is(urlbar.getText(), url, "Aweosmebar URL stayed the same"); :-(

stephend: fyi Bug 707358

stephend, I have that typo fixed in a patch I will upload to bug 711591. We still have one more patch to land for this bug, so I am reopening.

last patch landed on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/04ef7f564450

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/2ea7afaa47a9 - one of the two in that push was hitting "TypeError: Cc['@mozilla.org/android/bridge;1'] is undefined at :0" in desktop mochitest-other.

relanded on inbound, and it looks good this time: https://hg.mozilla.org/integration/mozilla-inbound/rev/b73c54dfb1d0