cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Grieve <agri...@chromium.org>
Subject Re: Android JUnit Tests Now Pass
Date Thu, 12 Feb 2015 02:41:32 GMT
Sorry for the scare. Maybe I wasn't clear, but I did not delete any failing
tests. I *fixed* the failing tests by having them wait on mutexes instead
of using Thread.sleep().

What I *did* delete:
- Duplicate tests
- Massive amounts of copy & paste (refactor from having one activity
per-test, to having all tests share an activity and passing the URL to load
via the Intent)
- Tests that had no assertions

Examples of deletes:
LifecycleTest:
 -
https://github.com/apache/cordova-android/commit/4358a0473094963b83335c683b43d7094aca6c44#diff-7753f0a08baabc682815c620354bb33c
 - It has no assertions (nothing was being tested)
PluginManagerTest:
 -
https://github.com/apache/cordova-android/commit/4358a0473094963b83335c683b43d7094aca6c44#diff-5fa78a4d2b5c99cd3d1b923edfd07ad6
 - It has only one test, and it's commented out.
CordovaTest:
 -
https://github.com/apache/cordova-android/commit/4358a0473094963b83335c683b43d7094aca6c44#diff-0b78d46f0e845439f01d054238a17afb
 - has only commented out tests
GapClientTest:
 -
https://github.com/apache/cordova-android/commit/4358a0473094963b83335c683b43d7094aca6c44#diff-d71ae4e49caa340447954c24f9670eba
 - Asserts the classnames of things. This is already covered by other tests
FixWebView:
 -
https://github.com/apache/cordova-android/commit/4358a0473094963b83335c683b43d7094aca6c44#diff-66f4f1b9fd4ceab56d347e88d574518b
 - Unused class
IntentUriOverrideTest
 - This one shouldn't have been deleted. Got me here! It's easy to put back
though (has only one assertion)


I spent all of this time because *I agree* there is a lot of value in
tests. I spent a lot of time to ensure that test app still runs in
stand-alone mode, and that no real test was lost in this change.

Please take some time to look at this before attacking it. I put a great
deal of care into it.

I'll wait before reverting the revert, but I don't see where the anger is
coming from. Didn't we just recently discuss the desire to clean up the
tests?





On Wed, Feb 11, 2015 at 5:28 PM, Joe Bowser <bowserj@gmail.com> wrote:

> I'm reverting this now, 37 tests with 4 failures is much better than roughy
> 20 tests with 0 failures.  (I didn't run the refactored tests, since
> there's no point if critical tests are missing).
>
> On Wed Feb 11 2015 at 2:01:51 PM Jesse <purplecabbage@gmail.com> wrote:
>
> > +1 Revert
> >
> > And please let's stop deleting what other people wrote just because we
> > don't recognize it. These things should require discussion.
> >
> > @purplecabbage
> > risingj.com
> >
> > On Wed, Feb 11, 2015 at 1:53 PM, Joe Bowser <bowserj@gmail.com> wrote:
> >
> > > I think we should revert this refactor.  With the new refactored tests,
> > > they may pass but we lost a lot of the useful tests that we once had
> and
> > > these new tests have no value.  I don't know why you took it upon
> > yourself
> > > to throw away all the JUnit tests that didn't pass, but that misses the
> > > point.  I would have rather had the old tests expanded upon instead of
> > just
> > > deleted on your personal whim.
> > >
> > > I honestly don't know what to say, I know that we have a terrible
> working
> > > relationship at best, but this actually is making the project worse
> > > intentionally for unknown reasons.  In fact, I would almost say that
> this
> > > is purely a malicious change driven by ego, since I can't see a
> technical
> > > reason for any of it.
> > >
> > > On Wed Feb 11 2015 at 1:36:19 PM Joe Bowser <bowserj@gmail.com> wrote:
> > >
> > > > I think there's a lot of value in the Unit Tests, having wrote the
> > > > majority of them initially.  If I wasn't dealing with everyone in my
> > > house
> > > > getting sick, I'd check to make sure these tests were still testing
> > what
> > > I
> > > > intended them to test, since we have a habit of losing the intent
> > behind
> > > > the test every time we do a refactor.
> > > >
> > > > Of course, if we're going to throw away the embedded WebView case,
> then
> > > > maybe there's not value after all.
> > > >
> > > > On Wed Feb 11 2015 at 1:12:29 PM Andrew Grieve <agrieve@chromium.org
> >
> > > > wrote:
> > > >
> > > >> Does travis provide Android emulators? I'd guess it'd be too slow
to
> > put
> > > >> on
> > > >> Travis. And honestly, there's still not a lot of value in the unit
> > tests
> > > >> atm.
> > > >>
> > > >> On Wed, Feb 11, 2015 at 3:12 PM, Murat Sutunc <
> muratsu@microsoft.com>
> > > >> wrote:
> > > >>
> > > >> > This is great news!
> > > >> > I've finally got the android travis enabled too. We have jshint
> and
> > > >> > jasmine test coverage on every commit now. (
> > > >> > https://travis-ci.org/apache/cordova-android/builds/50295748)
> > > >> >
> > > >> > Now that we're passing all junit tests, I think the next step
for
> us
> > > >> > should be to integrate junit tests with travis. What do you think?
> > > >> >
> > > >> > -----Original Message-----
> > > >> > From: agrieve@google.com [mailto:agrieve@google.com] On Behalf
Of
> > > >> Andrew
> > > >> > Grieve
> > > >> > Sent: Tuesday, February 10, 2015 7:14 PM
> > > >> > To: dev
> > > >> > Subject: Android JUnit Tests Now Pass
> > > >> >
> > > >> > Spent some time cleaning up the tests. Certainly they could be
> made
> > > even
> > > >> > better & made to test more things, but at least they pass
now :)
> > > >> >
> > > >> > Much of the change was deleting copy & paste, and deleting
> commented
> > > out
> > > >> > tests:
> > > >> > 53 files changed, 941 insertions(+), 2610 deletions(-)
> > > >> >
> > > >> > ------------------------------------------------------------
> > ---------
> > > >> > To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> > > >> > For additional commands, e-mail: dev-help@cordova.apache.org
> > > >> >
> > > >>
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message