fix web-runner #3

Open
wants to merge 24 commits into
from

Conversation

Projects
None yet
4 participants
@dominictarr
Collaborator

dominictarr commented Nov 7, 2010

found missing } which was preventing web-runner from compiling.

dominictarr added some commits Oct 16, 2010

writing tests, discovered that since runFile communicates over stdio,
logging in stdio breaks it...

beginning to rewrite runFile to use some other means of communication.

moved message functions into async_testing/lib/messages.js
lots of work on tests,
found a couple of problems with child,

tests that logged stuff was breaking reporting,
and you could make false positives by outputting a finish message (indicating 0 tests, which is success(!))

started working on another child process implementation, using http.

but realised that the messages must be queued, so that they arrive in order.
@bentomas

This comment has been minimized.

Show comment Hide comment
@bentomas

bentomas Nov 18, 2010

Can you post a gist of a file that logs to stdout and breaks runFile? I can't reproduce that. At this point runFile's implementation is very simple and straight-forward and I'm reluctant to make it do more unless it is absolutely necessary. I actually have a patch in the works that allows people running runFile to catch the stdout of a suite (looks like this https://gist.github.com/704636) to give suite runners full control over how a suite is being outputted. I think this is great, and don't want to have stdout automatically get printed if we can avoid it.

If stdout messages are getting confused as encoded messages then we should probably just make coded messages more unique.

Can you post a gist of a file that logs to stdout and breaks runFile? I can't reproduce that. At this point runFile's implementation is very simple and straight-forward and I'm reluctant to make it do more unless it is absolutely necessary. I actually have a patch in the works that allows people running runFile to catch the stdout of a suite (looks like this https://gist.github.com/704636) to give suite runners full control over how a suite is being outputted. I think this is great, and don't want to have stdout automatically get printed if we can avoid it.

If stdout messages are getting confused as encoded messages then we should probably just make coded messages more unique.

This comment has been minimized.

Show comment Hide comment
@dominictarr

dominictarr Nov 18, 2010

Owner

hi, i was getting a bunch of weird output occasionally

things like all the tests reporting "OK", but the end report saying there was problems.

here is a simple one,

https://gist.github.com/704800

Owner

dominictarr replied Nov 18, 2010

hi, i was getting a bunch of weird output occasionally

things like all the tests reporting "OK", but the end report saying there was problems.

here is a simple one,

https://gist.github.com/704800

This comment has been minimized.

Show comment Hide comment
@dominictarr

dominictarr Nov 18, 2010

Owner

also, since it depends on new lines,
if you use require('util').print("xxxxxx")

it will prevent the next message from getting through... which is surely onTestDone

... i have a test which checks that onTestStart, onTestDone, and onSuiteDone are all called correctly.
https://github.com/dominictarr/node-async-testing/blob/master/test/async_testing.options.async.js

you can break it by adding require('util').print("xxxxxx") into a test in the test/examples directory.

runFile IS simple, i got a bit carried away yesterday, trying to rewrite it with http.
at 3am i realized that i needed some way to queue the messages so that they arrive in the correct order.

I learnt much more about http than i knew before,
but I've also thought of a way to make messaging across stdout more secure.
first grab them with regex, so that they don't have to have their own line, and second,
wrap them in 'magic numbers' (a highly unlikely random string of digits, which will be passed to child,
so that messages can be distinguished from each other with astronomically low chances of collision.

Owner

dominictarr replied Nov 18, 2010

also, since it depends on new lines,
if you use require('util').print("xxxxxx")

it will prevent the next message from getting through... which is surely onTestDone

... i have a test which checks that onTestStart, onTestDone, and onSuiteDone are all called correctly.
https://github.com/dominictarr/node-async-testing/blob/master/test/async_testing.options.async.js

you can break it by adding require('util').print("xxxxxx") into a test in the test/examples directory.

runFile IS simple, i got a bit carried away yesterday, trying to rewrite it with http.
at 3am i realized that i needed some way to queue the messages so that they arrive in the correct order.

I learnt much more about http than i knew before,
but I've also thought of a way to make messaging across stdout more secure.
first grab them with regex, so that they don't have to have their own line, and second,
wrap them in 'magic numbers' (a highly unlikely random string of digits, which will be passed to child,
so that messages can be distinguished from each other with astronomically low chances of collision.

This comment has been minimized.

Show comment Hide comment
@dominictarr

dominictarr Nov 18, 2010

Owner

wow, another confusing thing is that
require('async_testing/lib/testing').runSuite(filename)
makes a stack overflow.

now that i have realised that it's meant be runSuite(require(filename)), I will be able to get a good night's sleep...

Owner

dominictarr replied Nov 18, 2010

wow, another confusing thing is that
require('async_testing/lib/testing').runSuite(filename)
makes a stack overflow.

now that i have realised that it's meant be runSuite(require(filename)), I will be able to get a good night's sleep...

@deepthawtz

This comment has been minimized.

Show comment Hide comment
@deepthawtz

deepthawtz Nov 24, 2010

/usr/local/lib/node/.npm/async_testing/9999.0.0-LINK-25de0aad/package/lib/web-runner.js:343
});

/usr/local/lib/node/.npm/async_testing/9999.0.0-LINK-25de0aad/package/lib/web-runner.js:343
});

@dbrock

This comment has been minimized.

Show comment Hide comment
@dbrock

dbrock Jan 3, 2011

You should really fix this immediately. It prevents the whole package from even loading.

dbrock commented Jan 3, 2011

You should really fix this immediately. It prevents the whole package from even loading.

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