cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Lorin Beer <lorin.beer....@gmail.com>
Subject Re: Refactoring the CLI tests
Date Fri, 08 Nov 2013 18:48:39 GMT
>What we're aiming for here is that we mock as little as is practical,

I think that kind of defeats the idea of unit tests in the first place, I
like the clean delineation between unit tests and integration tests, and it
sounds like both need to be improved. Mocking things at the boundary of the
CLI is in line with this.

>We'll do it first, and then solve the speed problems later if any arise.

agreed, and we can mitigate potential problems by keeping unit tests small
and within the boundaries of the CLI.

>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.

Yeah, mocking file system calls is a little goofy, especially considering
the scope of the CLI as a project. We can reasonably expect  a user to have
an HD, no?

My philosophy on testing is not to waste time optimizing tests, it's to
keep the tests small and lightweight. Each test deals with the smallest
unit of code or functionality possible. That's what makes them fast.
Integration tests deal with functionality that encompasses
inter-process/inter-project/network communication, and where latency is
expected.




On Thu, Nov 7, 2013 at 4:32 PM, Mark Koudritsky <kamrik@google.com> wrote:

> 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