add loads of tests, and make a few small changes where necessary to facilitate testing #42

Closed
wants to merge 11 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

dominictarr commented Jun 13, 2011

I've made all the product code changes in their own commits and left detailed notes in the commit message.

then all the tests.

I discovered several bugs, and if they where one line fixes I fixed them, but I left
'package.create : do not call require-analyzer if --noanalyze is set' failing because it was a bigger change.

to run tests:

npm install meta-test

meta-test test/*.js

(meta-test will also run vows tests)

cheers. Dominic

dominictarr added some commits Jun 13, 2011

[test][doc] a few small changes to facillitate testing. A few changes…
… in comments when I noticed they where out of date.
[doc] Can pass in any configuration setting on the command line via a…
… switch '--host localhost' for example. This was an already existing, but undocumented feature.
[fix] moved callback outside try block, which prevents callback being…
… called twice when there is throw inside callback.

[test] a few small changes to simplify testing.
[doc] update some comments.
[test] add mock based tests using meta-test (meta-test supports multi…
…ple test framworks, and will still run the vows tests with the command `meta-test test/*.js`)
Owner

indexzero commented Jun 13, 2011

I'm going to leave this open while we sort out the vows vs. node-mock thing. Lets schedule a few hours on Thursday to drill into this.

Contributor

dominictarr commented Jun 13, 2011

fair enough.

the reason I decided vows was not suitable was because vows the design of
vows is to set up a single async callback, which you then make assertions
about the result,

which is like that for a good reason, but unfortunately, since jitsu was
written before the tests, it doesn't have an architecture that suits vows.

Basically, in order to test the control flow of a command, (jitsu deploy is
a complicated one)
there is several rounds of io that all happens before the command is called
back, and there is no way to get coverage for the command without mocking
out all the IO.

jitsu deploy prompts the user, and then makes 7 http requests, and
possibly runs require-analyzer. you can test these requests in isolation,
because they are just the jitsu snapshot commands, but you can't get
coverage of the control flow without going through all the IO. so you have
to mock the prompt, and the requests.

so this doesn't seem to fit with the vows style, but possibly I
misunderstood some things about vows.

however, I'm beginning to suspect that mocking is an antipattern. I think
the deep problem is control flow is coupled to IO. I'm exploring using
Finite State Machines to handle control flow around IO, but I'm not
suggesting we use that now, it will require more experimentation on my own
time. https://github.com/dominictarr/fsm

On Mon, Jun 13, 2011 at 1:10 PM, indexzero <
reply@reply.github.com>wrote:

I'm going to leave this open while we sort out the vows vs. node-mock
thing. Lets schedule a few hours on Thursday to drill into this.

Reply to this email directly or view it on GitHub:
#42 (comment)

Owner

indexzero commented Jun 13, 2011

@dominictarr Actually, that's a common misconcept about vows:

the reason I decided vows was not suitable was because vows the design of
vows is to set up a single async callback, which you then make assertions
about the result,

This is actually incorrect. It is quite simple using the vows syntax to create tests with multiple asynchronous steps. This is what I will be showing you later in the week.

Contributor

dominictarr commented Jun 13, 2011

okay, cool. looking forward to it.

On Mon, Jun 13, 2011 at 2:07 PM, indexzero <
reply@reply.github.com>wrote:

@dominictarr Actually, that's a common misconcept about vows:

the reason I decided vows was not suitable was because vows the design of
vows is to set up a single async callback, which you then make assertions
about the result,

This is actually incorrect. It is quite simple using the vows syntax to
create tests with multiple asynchronous steps. This is what I will be
showing you later in the week.

Reply to this email directly or view it on GitHub:
#42 (comment)

+//to enable mocking the io we're gonna expose request here.
+
+Client.__requestHook = _request
+request = function (){
@dominictarr

dominictarr Jun 21, 2011

Contributor

missing var !

Owner

indexzero commented Jun 21, 2011

I will be leaving this open even now that it is pulled into the tests branch

Owner

indexzero commented Jun 23, 2011

@dominctarr Ready to merge the test branch in once:

test/client-test.js
test/package-test.js

Are updated to use vows.

Owner

indexzero commented Jun 25, 2011

This is a duplicate of #49

@indexzero indexzero closed this Jun 25, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment