Skip to content
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

Add UtTest_Teardown() to ut_assert #550

Closed
asgibson opened this issue Jul 24, 2020 · 9 comments · Fixed by #554 or #563
Closed

Add UtTest_Teardown() to ut_assert #550

asgibson opened this issue Jul 24, 2020 · 9 comments · Fixed by #554 or #563
Assignees
Labels
enhancement unit-test Tickets related to the OSAL unit testing (functional and/or coverage)
Milestone

Comments

@asgibson
Copy link
Contributor

Is your feature request related to a problem? Please describe.
I would like to have a "Teardown" that runs at the end of the unit tests test-runner file. Much like the UtTest_Setup runs at the start.

Describe the solution you'd like
Add a UtTest_Teardown() that is called after all the tests have run.

Describe alternatives you've considered
Not having it, which is the current state.

Additional context
This is a common feature with most unit test frameworks.

Requester Info
Alan Gibson
NASA Goddard Code 587

@asgibson asgibson added the unit-test Tickets related to the OSAL unit testing (functional and/or coverage) label Jul 24, 2020
@asgibson
Copy link
Contributor Author

I think this may be similar to #197, but I am not sure as my request is a 'Group' level Teardown. I am not sure exactly to what #197 is referring because I could interpret that to mean 'each individual unit test' (although this already exists so that does not seem likely).

@skliper
Copy link
Contributor

skliper commented Jul 27, 2020

Could you expand on what the requirements for this function should be, in comparison to what is already done?

@asgibson
Copy link
Contributor Author

I was looking for a function that happens at the end of a test-runner executable. The UtTest_Setup is a required function and where the UtTest_Add is used to add the tests, but because it runs before the tests it may also be used for any group setup functionality. The ut-assert framework then runs the tests, but I don't see anywhere to put "finalizing" statements or any other operation that I want to run at the end, after all the tests are complete.

@jphickey
Copy link
Contributor

Seems like this could be handled just by calling UtTest_Add() with only a teardown function (i.e. pass NULL for both setup and test, only teardown is non-NULL). Just add it last, and it can close whatever you opened in the setup.

Although it is trivial to call a UtTest_Teardown equivalent to UtTest_Setup, this will break existing tests that DON'T define this function, unless we do some static linker trickery which I prefer to avoid.

The other approach would be to have a dedicated function to register a teardown routine, akin to POSIX atexit(), which has the advantage that it could run even if a test aborts, but not sure its worth the complexity for minor benefit over the existing UtTest_Add().

@asgibson
Copy link
Contributor Author

While I appreciate that this would work, that seems like a hack. The teardown is not a test, so using UtTest_Add() to have a teardown is not syntactically correct. All I need is someone to come along and start adding tests at the end of a UtTest_Setup after my teardown and its broken.

Also, what if I want to randomize the ordering of my tests (which helps to ensure test independence)? I was able to do that with the old ut_assert, but I have not yet implemented it for the new one. This would not allow for that to be done (unless of course it specifically looks for a "teardown" test and always puts it at the end). This is not a good solution.

I don't understand the argument about breaking current tests because this is what updating the ut_assert did to the old tests in the first place. Are we still not in the transition period where these kinds issues are being worked out? I reiterate that the teardown is a common feature in unit testing frameworks that I have used

@asgibson
Copy link
Contributor Author

Could we have a UtTest_AddTeardown? Thus, if it existed it could be run, if not then don't? This could make it optional, where it would not matter where it appeared in the UtTest_Setup function and also separate it from the tests.

@jphickey
Copy link
Contributor

While I appreciate that this would work, that seems like a hack

Not entirely - functions added with UtTest_Add() that are either setup-only or teardown-only (i.e. no actual test) do not increment the test counter - which suggests to me that this was perhaps the intended use-case even though it isn't specifically documented that way.

Could we have a UtTest_AddTeardown? Thus, if it existed it could be run, if not then don't? This could make it optional, where it would not matter where it appeared in the UtTest_Setup function and also separate it from the tests.

Right, that's what I was alluding to with the "other approach" on my previous comment - basically a version of UtTest_Add() only has teardown and adds at the end of the list such that they are always run after the normal tests.

Totally doable, but given that today tests are always run in a constant order, it doesn't really add anything that can't already be done - just cleaner. It does future-proof, though, in that if we do add a test order randomization feature in the future (might be nice) then it is important that these are still kept last - so that might be justification enough to do this.

If we go that route, I would suggest doing it for Setup too - i.e. allow additional setup-only routines to be registered, which are executed first (in order), followed by tests (possibly randomized in a future version), followed by teardowns (in order).

That isn't hard to implement, so if others are in agreement then I'm happy to implement it.

@skliper - thoughts on that?

@asgibson
Copy link
Contributor Author

My vote is for that approach. Thank you for the suggestion.

@skliper
Copy link
Contributor

skliper commented Jul 28, 2020

Yes please, this approach is exactly what I assumed was being asked for (registration of function to run last). Registration of setup (runs first) makes sense also.

@jphickey jphickey self-assigned this Jul 31, 2020
jphickey added a commit to jphickey/osal that referenced this issue Jul 31, 2020
Clean up and Refactor the internal list storage structures to support
having multiple list heads/groups.  Define a separate list/group for
setup, normal test, and teardown.

The existing UtTest_Add() routine maps to the normal test group.
Added a UtTest_AddSetup() and UtTest_AddTeardown() routine that maps
to the setup and teardown group, respectively.

The existing UtTest_Run() routine then merges the groups and executes
the full suite of tests.

This allows separate, dynamic registration of test setup and teardown
routines which are executed before and after the normal test routine,
which can create and delete any global/common test prerequisites.
@astrogeco astrogeco added this to the 6.0.0 milestone Aug 5, 2020
astrogeco added a commit that referenced this issue Aug 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement unit-test Tickets related to the OSAL unit testing (functional and/or coverage)
Projects
None yet
4 participants