cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michal Mocny <mmo...@chromium.org>
Subject Re: plugin-test-framework PRs
Date Fri, 01 Aug 2014 17:35:50 GMT
On Fri, Aug 1, 2014 at 1:19 PM, Staci Cooper <staci.mcl@gmail.com> wrote:

> I've noticed some problems I'd like to bring up.
>
> 1)
>
> https://github.com/apache/cordova-mobile-spec/blob/master/www/index.html#L44
> This line breaks on windows phone when the query property is used.
>

1. was a recent change.  I'm happy to revert / would love a fix if you have
easy access to test on windows phone.


>
> 2) My pull requests include the manual tests for contacts, device-motion,
> and geolocation on top of existing automated tests, which I did not change.
> I just tested those automated tests and they are breaking on WP; there are
> 2 tests failing in geolocation which shouldn't and 3 in contacts.
> Device-motion is more severe and is breaking the entire test suite on WP. I
> can start looking into those.
>

Thank you for bringing that up.  Jesse has been awesome at helping test
this stuff on windows so far, perhaps I'll ask him (and bryan/josh for BB)
to confirm before landing these PR's.


>
> 3) I'm not sure how to make the PRs to mobile-spec for removing tests
> without requiring them to be merged in a specific order or causing merge
> conflicts around pages like index.html. My suggestion is that we make a PR
> for each plugin which removes the test files but leaves the buttons/script
> elements in to avoid merge conflicts, and just add a warning to mobile-spec
> that some links will be broken while we go through the porting process.
> Then when everything is done we can remove the links in a final PR.
>

I've run into these issues before.  I usually resolve the merge conflict
locally in the merge commit.  Usually they aren't hard.  If the conflicts
are actually hairy, I'm happy to remove the finally bits in bulk as you
suggest.


>
> -Staci
>

Thanks for all the effort here!


>
>
>
> On Fri, Aug 1, 2014 at 3:55 AM, Martin Gonzalez <
> martin.c.glez.glez@gmail.com> wrote:
>
> > Hey Michal,
> > I've followed up your instructions to easily integrate the test along
> with
> > plugin-test-framework, as you can see I have spammed a little bit the dev
> > list with PRs, anyway here's the list of PRs, that includes File, Media,
> > Battery-Status and Vibration:
> > https://github.com/apache/cordova-plugin-file/pull/62
> > https://github.com/apache/cordova-plugin-media/pull/21
> > https://github.com/apache/cordova-plugin-battery-status/pull/15
> > https://github.com/apache/cordova-plugin-vibration/pull/19
> >
> > And the removed tests at mobile-spec:
> > File: https://github.com/apache/cordova-mobile-spec/pull/86
> > Media: https://github.com/apache/cordova-mobile-spec/pull/85
> > Battery: https://github.com/apache/cordova-mobile-spec/pull/87
> > Vibration: https://github.com/apache/cordova-mobile-spec/pull/88
> >
> > Also a few days ago, I created a PR against plugin-test-framework, just
> > some little changes to the css, all details on the description:
> > https://github.com/apache/cordova-plugin-test-framework/pull/3
> >
> > Everything tested with createmobilespec, and android device.
> >
> > If you can take a look to any of this, it would be awesome.
> >
> > Thanks,
> > Best Regards,
> > Martin
> >
> > 2014-07-30 10:44 GMT-05:00 Staci Cooper <staci.mcl@gmail.com>:
> >
> > > Sounds good, I'll get started.
> > >
> > >
> > > On Wed, Jul 30, 2014 at 11:33 AM, Michal Mocny <mmocny@chromium.org>
> > > wrote:
> > >
> > > > Things to help move this along:
> > > >
> > > > - Move test directories to "tests" and add nested plugin.xml, that
> > would
> > > be
> > > > a great.  You can look at an example here:
> > > > https://github.com/apache/cordova-plugin-device/tree/master/tests
> > > > - Test the PR locally by running the mobilespec create script to make
> > > sure
> > > > the plugin tests install fine and run well.  (The part that adds
> > > new-style
> > > > tests is here:
> > > >
> > > >
> > >
> >
> https://github.com/apache/cordova-mobile-spec/blob/master/createmobilespec/createmobilespec.js#L248
> > > > )
> > > > - Double check to make sure we have covered all tests currently in
> > > > mobilespec, and submit PR to remove the old tests from mobilespec
> app.
> >  I
> > > > would not like to land the new tests without removing the old tests
> as
> > > > well.
> > > >
> > > > Thanks a bunch!
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > On Wed, Jul 30, 2014 at 11:25 AM, Staci Cooper <staci.mcl@gmail.com>
> > > > wrote:
> > > >
> > > > > Just want to bump this thread.
> > > > >
> > > > > Michal -- that all sounds great. Let me know if there's anything
I
> > can
> > > do
> > > > > to help with integration, or if the IBM committers can help with
> > > merging.
> > > > > Should we go ahead and add nested plugin.xml files to the PRs?
> > > > >
> > > > > Also, for reference: I tested all of my PRs on wp8 with a
> comparison
> > > with
> > > > > mobile-spec's behavior, as did Martin with his. Edna and Martin
> have
> > > also
> > > > > been testing on ios and android respectively.
> > > > >
> > > > >
> > > > > On Tue, Jul 22, 2014 at 9:52 AM, Michal Mocny <mmocny@chromium.org
> >
> > > > wrote:
> > > > >
> > > > > > Just a quick update since its been a few days.
> > > > > >
> > > > > > - Mobile-spec now builds with new-style tests bundled in (there
> is
> > a
> > > > > button
> > > > > > to launch this on the first screen), and the createmobilespec.sh
> > > script
> > > > > > will automatically install plugin tests.
> > > > > > - Last week I moved device tests from mobile-spec over to plugin
> > > tests,
> > > > > and
> > > > > > modified the file plugin tests to use a nested plugin.xml.
> > > > > > - File and FileTransfer tests are tightly coupled in mobile-spec,
> > and
> > > > > > should be removed together.  I have this mostly finished locally,
> > > but..
> > > > > > - I want to set up ci.cordova.io to include results of new-style
> > > tests
> > > > > > before ripping out huge portions of mobile spec, which is what
> I'm
> > > > doing
> > > > > > now.
> > > > > >
> > > > > > Few notes:
> > > > > > - The PR's have created a "test" folder, but I had written
> scripts
> > to
> > > > > > expect "tests" folder.  Its easy to change or just accept both,
> > but I
> > > > > > wonder if we should settle on a single convention.  Total
> bikeshed
> > > > topic,
> > > > > > so I'll just pick one.
> > > > > > - PR's for new tests seem to be well isolated from each other
> > (unlike
> > > > > > mobilespec).  Aka you can run FileTransfer tests without File
> > tests.
> > > > >  Good
> > > > > > job!
> > > > > >
> > > > > > -Michal
> > > > > >
> > > > > >
> > > > > > On Wed, Jul 16, 2014 at 5:02 PM, Michal Mocny <
> mmocny@chromium.org
> > >
> > > > > wrote:
> > > > > >
> > > > > > > Sure, I was actually already planning to take a look this
week.
> >  I
> > > > was
> > > > > > > working on mobile-spec today and have *also* ported device
> tests
> > :P
> > > >  I
> > > > > > > should have looked at the PR's first!  Will start these
> tomorrow.
> > > > > > >
> > > > > > > The plan for mobile spec was just to have a transition
path, by
> > > > adding
> > > > > a
> > > > > > > link to the old mobile-spec app to open the new-style-tests
> > > harness.
> > > > > >  Then,
> > > > > > > as we move tests from mobile-spec to new-style, we should
> remove
> > > the
> > > > > old
> > > > > > > tests from mobile-spec.  Doing it this way means cordova
> > committers
> > > > > have
> > > > > > a
> > > > > > > single place to run all tests, and when mobile-spec is
100%
> > > > completely
> > > > > > > deprecated, then we can just switch the start page with
no
> change
> > > to
> > > > > > > committers.
> > > > > > >
> > > > > > > Thanks for your work here!
> > > > > > >
> > > > > > > -Michal
> > > > > > >
> > > > > > >
> > > > > > > On Wed, Jul 16, 2014 at 4:38 PM, Staci Cooper <
> > staci.mcl@gmail.com
> > > >
> > > > > > wrote:
> > > > > > >
> > > > > > >> Hi all,
> > > > > > >>
> > > > > > >> Several of us at IBM have been working on porting tests
from
> > > > > mobile-spec
> > > > > > >> to
> > > > > > >> the plugin-test-framework. I believe we have pull requests
out
> > for
> > > > all
> > > > > > of
> > > > > > >> the automated tests; we also have the manual tests
ported and
> > are
> > > > just
> > > > > > >> wrapping up testing on ios/android, so those additional
pull
> > > > requests
> > > > > > >> should be out by tomorrow evening.
> > > > > > >>
> > > > > > >> Can we get these reviewed soon to avoid getting out
of sync
> with
> > > > > mobile
> > > > > > >> spec? Michal, if you have time to take a look that
would be
> > > > fantastic;
> > > > > > we
> > > > > > >> can also get some of the IBM committers to help if
needed.
> > > > > > >>
> > > > > > >> Related note: I see that Michal added the new style
tests to
> > > > > > mobile-spec,
> > > > > > >> but is the plan for plugin-test-framework to be supplementary
> to
> > > > > > >> mobile-spec tests? It seems there would be problems
keeping
> them
> > > in
> > > > > > sync.
> > > > > > >> If it hasn't already been suggested, I propose removing
> > > mobile-spec
> > > > > > tests
> > > > > > >> as the ported tests get merged in. The mobile-spec
project
> would
> > > > > > >> eventually
> > > > > > >> become a shell project with all tests run through
> > > > > plugin-test-framework
> > > > > > >> and
> > > > > > >> installed plugins.
> > > > > > >>
> > > > > > >> Thanks,
> > > > > > >> Staci Cooper
> > > > > > >>
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> >
> >
> > --
> > Regards,
> > Martin Gonzalez
> >
>

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