cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Koudritsky <kam...@google.com>
Subject Re: Refactoring the CLI tests
Date Fri, 08 Nov 2013 00:32:37 GMT
My main frustration with the current state of the tests is how fragile some
of them are with respect to even minor changes in the CLI. As I see it,
this is closely related to mocking the file system calls. CLI is all about
massaging the files around, most of the state we care about lives on the
file system. So once we mock the fs calls and can't check for existence and
contents of files, we don't have much to check for in the tests, so we
start checking implementation details like specific function calls and then
it gets fragile. As far as I understand, the kind of relationship that CLI
has with the file system is not so common with other Node.js based projects
and the distinction between unit and integration tests is somewhat blurred
here.

Anyway, as Braden said, we'll see what parts of the end-to-end tests will
be slow and work on speeding some of them up and mocking others away. I've
fallen prey to premature optimization traps way too many times by now :)

- Mark


On Thu, Nov 7, 2013 at 3:06 PM, Braden Shepherdson <braden@chromium.org>wrote:

> What we're aiming for here is that we mock as little as is practical, and
> ideally only mock things at the boundaries of CLI: calls to the platform
> scripts, some calls to Plugman (generally we want to let these end-to-end
> tests call the real Plugman, but there are exceptions).
>
> We'll do it first, and then solve the speed problems later if any arise.
> There are some options here, like I mentioned above: Use a fake fs
> implementation[1] or a ramdisk with (much!) better performance than the
> real disk. And just make CLI faster in general.
>
> Braden
>
> [1] https://github.com/eldargab/node-fake-fs
>
>
> On Thu, Nov 7, 2013 at 2:57 PM, Anis KADRI <anis.kadri@gmail.com> wrote:
>
> > The reason why every FS call is mocked is speed but speed is
> > subjective in my opinion. Given the features of CLI, my opinion is
> > that anything < 1 minute is acceptable. When I run tests, I am not
> > actively watching them execute.
> > I think the only calls that should be mocked are network calls because
> > you should be able to run tests offline.
> > I think we should convert (trash?) existing tests back to what they
> > were instead of adding more end-to-end tests. We should also pay more
> > attention de Windows.
> >
> > On Thu, Nov 7, 2013 at 10:31 AM, Braden Shepherdson <braden@chromium.org
> >
> > wrote:
> > > Alright, Mark and I have discussed this further, and we will be
> beginning
> > > the effort with some end-to-end tests that will supplement the existing
> > > tests.
> > >
> > > To some extent this is duplicating things that go on in the CI, since
> it
> > > checks out various plugins using the tools. But we think it's still a
> > > worthwhile effort since it will make it much easier to catch any
> problems
> > > at commit time.
> > >
> > > The main pain point in the existing tests that we'll want to fix is
> their
> > > fragility. The worst offenders here are the platform parsers,
> especially
> > > wp7+8. So Steve, if you're looking for ways to help, I'd suggest
> looking
> > at
> > > those (wp in particular, but all of them). These are some of the worst,
> > > most vacuous tests we have. They're not providing sample inputs and
> > > checking the outputs, they are checking that the right functions get
> > > called. I propose that they should be operating on a real, checked-in,
> > > example project, in the spec/fixtures directory. The tests should run
> the
> > > parsers, and make sure all the files are in the right places and have
> the
> > > right contents.
> > >
> > > Braden
> > >
> > >
> > > On Thu, Nov 7, 2013 at 1:02 PM, Steve Gill <stevengill97@gmail.com>
> > wrote:
> > >
> > >> I don't think we should scrap the current tests. I am totally in favor
> > of
> > >> having new end to end tests. We need to catch regressions better.
> > >>
> > >> Braden, let me know how I can help.
> > >>
> > >>
> > >>
> > >> > On Nov 7, 2013, at 9:02 AM, Michal Mocny <mmocny@chromium.org>
> wrote:
> > >> >
> > >> > Discussing locally with Braden.. these tests seem to be testing
> > internal
> > >> > details instead of expected functionality.  Its quite common for
> valid
> > >> > patches to break the tests and invalid patches to leave the tests
> > >> passing.
> > >> > There have been several occurrences recently where things landed
> even
> > >> > though "cordova create" or "plugin add" were hosed, which seems
> fairly
> > >> > fundamental to keep working ;)
> > >> >
> > >> >
> > >> >> On Thu, Nov 7, 2013 at 11:57 AM, Michal Mocny <mmocny@chromium.org
> >
> > >> wrote:
> > >> >>
> > >> >> +1 to testing end-to-end, but I thought thats what BuildBot/Medic
> > does?
> > >> >> Likely we want to ship those end-to-end test scripts so users
can
> > test
> > >> >> changes locally, but I think the tests are are inside CLI now
are
> > meant
> > >> to
> > >> >> be unit tests, which yes, have very limited usefulness in
> isolation,
> > but
> > >> >> perhaps shouldn't be entirely replaced?  If they really are useless
> > (not
> > >> >> sure) then they should be fixed/removed.
> > >> >>
> > >> >>
> > >> >> On Thu, Nov 7, 2013 at 11:40 AM, Braden Shepherdson <
> > >> braden@chromium.org>wrote:
> > >> >>
> > >> >>> The CLI tests are bad. I propose making them better.
> > >> >>>
> > >> >>> The tests are bad for two reasons:
> > >> >>> 1. They're fragile because the tests depend on exactly the
right
> > >> functions
> > >> >>> being called, sometimes in the right order.
> > >> >>> 2. They don't test what we really want, which is that projects
get
> > >> created
> > >> >>> and all the files are in the right places.
> > >> >>>
> > >> >>> I propose letting the tests actually run filesystem and related
> > calls,
> > >> >>> instead of always mocking them out. In the simplest form,
that
> means
> > >> >>> running them on the real filesystem. If that's too slow, we
can
> > >> >>> investigate
> > >> >>> other alternatives, like using a ramdisk, or using that emulated
> fs
> > >> that
> > >> >>> runs everything in RAM inside Node.
> > >> >>>
> > >> >>> Mark and I are planning to work on this, starting with the
CLI
> > tests.
> > >> The
> > >> >>> Plugman tests are better in this regard, but could probably
stand
> > to be
> > >> >>> really called as well.
> > >> >>>
> > >> >>> Any thoughts or objections?
> > >> >>>
> > >> >>> Braden
> > >> >>
> > >> >>
> > >>
> >
>

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