Skip to content

Conversation

@jgranick
Copy link
Contributor

@jgranick jgranick commented Nov 13, 2018

I'm opening this pull to help start a discussion around the TiVo contributions.

In our use of munit, we've reached the point (on the OpenFL team) of wanting data provider support, and (more importantly) requiring better async testing. The changes made by the TiVo lead to proper asynchronous testing within Lime and OpenFL, which is a huge plus to us.

There are two issues that I believe led to the proposal being frozen:

1.) It changes the signature of asynchronous test methods to not require a factory parameter.

2.) It's old, and didn't merge properly anymore

We could ask @sganapavarapu1 about all the reasons behind the API change, but at first guess, I would say that this might fix timing for when asynchronous methods are called, or makes it easy to quickly @AsyncTest a method.

(EDIT: Oh, I see the DataProvider support, perhaps the async factory parameter also muddies how data providers work, if you mixed it with async: https://github.com/sganapavarapu1/MassiveUnit/blob/master/test/massive/munit/DataProviderTest.hx#L65-L85)

Regardless, this raises another question about munit. The comments mentioned "munit 2 being released," but I'm not sure if we can expect this to occur. Is "munit 2" a reality? Would it be possible to work as a community to finish or improve this unreleased version?

Otherwise, could we consider whether this (or other) breaking changes make sense?

Thank you guys

sganapavarapu1 and others added 10 commits June 4, 2013 16:02
TestClassHelper - added support for multiple
before/after/beforeclass/afterclass methods called in proper inheritance
order, support for dataprovider annotation
TestResult - added support for optional arguments passed via
dataprovider
TestRunner - reworked async support to not require a parameter to the
function by making the active runner a static. if 'nme' is defined, made
tests run without blocking so that nme can process the main loop.
various changes to support multiple before/after methods.  various fixes
to async test handling, including calling async timeout handler.
AsyncDelegate, AsyncFactory - added timeout handler support
PrintClientBase - improved error reporting
TestClassHelperTest, TestClassStub - added unit tests for multiple
before/after method annotations being called in proper inheritance
order.
UnexpectedAsyncDelegateException - new exception for unexpected async
requests
Async - new global class for making Async requests
TestClassStubSuper - unit test class for testing super class
before/after annotations.
1. TestRunner.hx : has these enhancements,
    a) chained async test support
    b) simultaneous multiple async test support
    c) allowing exceptions to occur in @BeforeClass, @afterclass, @before, @after methods
2. TestClassHelper.hx : Fix: allows static scoped data providers
3. AsyncDelegate.hx : Fix: does not call time-out handler if the async test is already cancelled
4. New files:
    a) ChainedAsyncTest.hx - unit tests for 1(a)
    b) ParallelAsyncTest.hx - unit tests for 1(b)
    c) PrePostActionAssertsTest.hx - unit tests for 1(c)
    d) DataProviderTest.hx - unit tests for data provider feature
                             has an hamcrest matcher that should probably go into hamcrest repo
    e) DataProviderUsageExample.hx - just an example source for data provider usage
5. TestSuite.hx - updated suite to include 4(a), 4(b), 4(c), 4(d)
6. All other files - Renamed Async.asyncHandler to Async.handler, or removed unapplicable copyright
7. Removed haxelib.json
@elsassph
Copy link
Contributor

There won't be a munit 2. This project was shelved a while ago and instead this library has been significantly cleaned up relatively recently and I'd welcome further improvements, provided it doesn't cause serious API changes.

@elsassph
Copy link
Contributor

@SlavaRa what do you think?

@jgranick
Copy link
Contributor Author

I'm open to looking at migrating the async syntax back (to avoid API changes), though trying to think of the advantages of the static approach, apart from keeping the function parameters clean.

@jgranick
Copy link
Contributor Author

jgranick commented Apr 4, 2019

Closed in favor of: #164

@jgranick jgranick closed this Apr 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants