Also - maybe leave the inflate test using AndroidWebView, and make the other ones create dynamically?
On Tue, Jan 20, 2015 at 2:59 PM, Andrew Grieve <[email protected]> wrote: > gotcha. I've now got tests building & running with gradle (painful > learning curve...). I'm going to pull in the changes from this PR that fix > the compile, but will leave the rest. > > Junwei - do the tests pass for you? I certainly can't get most of them to > (running on KitKat) :( > > On Tue, Jan 20, 2015 at 1:00 PM, Joe Bowser <[email protected]> wrote: > >> It's an issue because this test was supposed to be testing the embedding >> of >> a WebView via layout. That's why the inflate exists in the code. By >> creating the webview directly, it's not actually testing anything of >> value. We actually lose test coverage with this change. >> >> >> On Tue Jan 20 2015 at 9:58:19 AM Andrew Grieve <[email protected]> >> wrote: >> >> > Nice. I've got a gradle file that now at least builds the application, >> so >> > not far off of having it run the tests. >> > >> > Totally on board with re-writing the unit tests (they have an >> unfortunate >> > number of Thread.sleep() in them). But this PR at least makes them >> compile >> > on 4.0.x. Maybe add some comments to the PR where you think it's not >> > testing AndroidWebView? Just had another look and the only thing that I >> > think changes is that it's creating the webview in code rather than by >> > inflating. >> > >> > CordovaInterface hides the fact that AndroidWebView is a View anyways, >> so I >> > don't think Crosswalk not being a view should make a difference (you >> just >> > call .getView() in both cases). >> > >> > On Tue, Jan 20, 2015 at 12:40 PM, Joe Bowser <[email protected]> wrote: >> > >> > > It was easier to get the tests to run in Android Studio than it was to >> > get >> > > Cordova itself to run. You just import them in as an Eclipse project >> and >> > it >> > > just works. I was surprised when I did this a few days ago. >> > > >> > > I don't like this pull request because it removes all tests of the >> > > AndroidWebView component itself, and instead makes the tests conform >> to >> > the >> > > way that XWalk is designed. I think that we need a full re-write of >> the >> > > JUnit tests, but that can wait until after 4.0.x is released. >> > > >> > > On Tue Jan 20 2015 at 9:30:14 AM Andrew Grieve <[email protected]> >> > > wrote: >> > > >> > > > I think Junwei is saying that he has got them to work in his PR. >> > > > >> > > > I looked at this a while ago, but couldn't figure out how to get the >> > > tests >> > > > to run in the Android Studio / Gradle world. I'll have another >> crack at >> > > it. >> > > > >> > > > On Tue, Jan 20, 2015 at 8:37 AM, Joe Bowser <[email protected]> >> wrote: >> > > > >> > > > > The tests don't work with Crosswalk because Crosswalk's main class >> > > > doesn't >> > > > > inherit from a view. This is why we had to change the >> CordovaWebView >> > > > from >> > > > > being a class to being an Interface in the first place. I don't >> > think >> > > > > there is a way for these tests to work with Crosswalk because of >> this >> > > > > incompatibility. I don't think there is a way to re-use these >> tests >> > > > > because of this fundamental change. >> > > > > On Tue Jan 20 2015 at 5:11:54 AM Fu, Junwei <[email protected]> >> > > wrote: >> > > > > >> > > > > > Hi, >> > > > > > >> > > > > > I pulled cordova-android 4.0 branch, and running JUnit test in >> > /test >> > > > > > directory, but there are compiled error as below, and I want >> reuse >> > > the >> > > > > > JUnit tests to test Crosswalk pluggable webView, so I request >> a PR >> > > > > > https://github.com/apache/cordova-android/pull/140, could >> someone >> > > help >> > > > > me >> > > > > > to review and merge it. >> > > > > > >> > > > > > /test/menus.java:37: error: method registerForContextMenu in >> class >> > > > > > Activity cannot be applied to given types; >> > > > > > [javac] super.registerForContextMenu(super.appView); >> > > > > > reason: actual argument CordovaWebView cannot be converted to >> View >> > by >> > > > > > method invocation conversion >> > > > > > >> > > > > > test/splashscreen.java:33: error: method loadUrl in class >> > > > CordovaActivity >> > > > > > cannot be applied to given types; >> > > > > > [javac] >> > > > > >> super.loadUrl("file:///android_asset/www/splashscreen/index.html", >> > > > > > 2000); >> > > > > > reason: actual and formal argument lists differ in length >> > > > > > >> > > > > > Thanks, >> > > > > > Junwei. >> > > > > > >> > > > > > ------------------------------------------------------------ >> > --------- >> > > > > > To unsubscribe, e-mail: [email protected] >> > > > > > For additional commands, e-mail: [email protected] >> > > > > > >> > > > > >> > > > >> > > >> > >> > >
