-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DataProvider support, async fixes, headless AIR output #164
base: master
Are you sure you want to change the base?
Conversation
This has been updated with minor fixes. Thanks guys 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a significant PR!
src/massive/munit/TestRunner.hx
Outdated
}); | ||
runThread.sendMessage(Thread.current()); | ||
Thread.readMessage(true); | ||
#if (!lime && !nme && (neko || cpp || java)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason to exclude lime
and nme
?
Could it be enabled by a more non project-specific directive?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I didn't see these comments!
I believe this code is to prevent applications which have no main loop from exiting immediately.
I agree it would be nice if this was a little more generic, but there's no certain way to say that you're using a framework that is aware of munit and can provide a main loop. I guess an munit-specific define that needs to be added would be a fair compromise
override public function print(value:Dynamic) | ||
{ | ||
super.print(value); | ||
#if(neko || cpp || java || cs || python || php || hl || eval) | ||
Sys.print(value); | ||
#elseif (flash && air) | ||
flashOutput += value; | ||
while (flashOutput.indexOf("\n") > -1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not tracing as-is?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the issue is that Flash trace supports only one line at a time
{ | ||
actualValue *= L1_RESPONS_FACTOR; | ||
|
||
t2 = Async.handler ( this, forbiddenResponseHandler, 250, failingTimeOutHandler ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
About the asymetry between factory.createHandler
(explicit) and Async.handler
(implicit): could we instead have a factory.createAsyncHandler
which would be passed the factory
reference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I thought I removed the Async.handler
cases. Your suggestion sounds like a good compromise
I think the goal (originally) was to allow switching from sync to async without having to add an additional argument for the factory, but that change broke backward compatibility and prevented the pull request from ever being merged. I'm fine with leaving the factory argument in to get some of the other fixes/improvements we need
@jgranick sorry a recent merge is causingf a conflict - probably nothing serious. |
I just resolved the conflict, thanks 😄 |
This pull request merges DataProvider support, async test fixes and other improvements from the TiVo pull request here: #58
It also merges headless Adobe AIR support from here: #159
This improves upon the previous pull request (#161)
I have not included a rebuild of "index.n" or "run.n" in case this makes it easier to merge.
I have removed the
massive.munit.Async
class that was added in the original TiVo pull request and have changed it back to the originalfactory:AsyncFactory
pattern in order to prevent breaking the API.