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 CPPUnit Testing Framework #164

Merged
merged 29 commits into from
Sep 9, 2015
Merged

Add CPPUnit Testing Framework #164

merged 29 commits into from
Sep 9, 2015

Conversation

pbauman
Copy link
Member

@pbauman pbauman commented Aug 24, 2015

I've changed my mind about CPPUnit and am very much desiring the ease-of-adding new tests to the testing suite, especially in light of #163 etc. where we'll need to add more coverage.

I'd like to eventually migrate all unit tests (and regression tests?) to this framework. For now, I'm just trying to lay the groundwork to make adding new tests easy.

This is not complete yet for this PR, but the structure is there (using just the Arrhenius rate test) so that we can have a discussion about the setup.

My thinking was as follows:

  • There are "standard" tests that don't require any external dependencies (modulo CPPUnit...). There's a program standard_unit_tests that will run all these.
  • Each of the external dependencies (e.g. Eigen, VexCL, etc.) will have their own program to run. Currently, this PR has the eigen_unit_tests.

I've tried to style the tests to be as reusable as I can, but I'm sure there are parts I could do better. E.g. in the Eigen tests, I reuse the ArrheniusRateVectorTestBase class. So it's header only and then we just compile the sources needed for each in their own directory, e.g. standard_unit/arrhenius_rate_valarray_test.C and eigen_unit/arrhenius_rate_eigen_test.C.

The main reason I went this route is that it makes it clear what tests we're skipping and what we're not. This is as opposed to ifdef'ing out the tests and only have one-cppunit-program-to-rule-them-all. Thoughts on this setup?

If folks are OK with this setup, I'll drop the MetaPhysicL, VexCL, and GSL ones in and consider this PR complete for laying the ground work. Then we can migrate the existing tests in our spare time and add new ones to the CPPUnit framework.

"standard" means no other optional libraries are needed. We'll
setup separate programs for other tests that require optional
libraries so we can see what's skipped and what's not from the
`make check` report.

This commit only has the (scalar) Arrhenius rate test. We'll start
migrating some of the other tests now.
This is so we can reuse them for the vector version of the test.
Only valarray so far. Idea will be to have separate programs for
each of the optional libraries and drop those tests in there so we
can see what we're skipping from `make check`.

I've tried to set it up to adhere to DRY as much as possible.
Have file actually indicate valarray instead of ambiguous vector.
That way we can later add arrhenius_rate_eigen_test, for example.
And the Arrhenius-Eigen-vector unit test.
@pbauman
Copy link
Member Author

pbauman commented Aug 24, 2015

Discussion related to separating tests for dependencies happened offline with @roystgnr concluding

Yeah, that's a fine idea.

So I will proceed with this.

The cppunit config program doesn't split them up, but it's more
convenient on Mac to put them in LDFLAGS, otherwise it tries to find
the Macports version of CPPUnit and it's annoying.
And ArrheniusRate VexCL unit tests.
Includes ArrheniusRate vector test for MetaPhysicL
We have all that coverage (and more actually) in all the new CPPUnit
based tests so it's no longer needed.
Have started GSLSpliner unit test, checkpointing work on it.
Currently testing we exactly fit/eval constants, linears.
I want to construct and analytic case for the cubic function
that satifies the natural boundary conditions so that we have
a much better test than we had before.

template<typename PairScalars>
class ArrheniusRateVectorTestBase : public ArrheniusRateTestHelper</*Scalar*/typename Antioch::value_type<PairScalars>::type>,
public ReactionRateVectorBaseTest<Antioch::ArrheniusRate</*Scalar*/typename Antioch::value_type<PairScalars>::type>,PairScalars>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a class naming inconsistency? *TestBase versus *BaseTest.

@dmcdougall
Copy link
Contributor

Can we enable doxygen for the test classes? It'd be easier to see the class hierarchy that way.

GSL cubic splining uses natural conditions by default, so I
constructed a cubic Hermite function with f''(xmin) = f''(xmax) = 0.
Replaced by the GSL CppUnit tests.
Need data was Scalar, but operator() was on PairScalars
Within the CppUnit framework. Instead of splitting up the tests further
I just put all the different vector types in there knowing that
we'll see if Eigen, VexCL, etc. are skipped or not from other tests
so we'll know implicitly whehter they're being skipped in the GSL tests
too or not.
Made CppUnit-based testing classes more consistent with the naming.
Thanks @dmcdougall
In preparation for putting the tests in their own namespace.
In particular so we can see the CppUnit testing hierarchies.
@pbauman
Copy link
Member Author

pbauman commented Sep 4, 2015

All the optional library CppUnit structure is there now. In the last two commits, I turned on all the optional libraries when Doxygen gets built and then put all the testing in a separate namespace (AntiochTesting). I was hoping the latter might help organize the documentation better, but it didn't help all that much, IMHO.

Thoughts on that? I was thinking maybe an alternative would be to opt-in at configure time to have the testing documentation built and by default it's not.

Regardless I've addressed the other comments (thanks @dmcdougall) and the testing infrastructure is there so this is ready to go barring comments on the documentation (and anything else that comes up if @roystgnr takes a look).

Use AntiochTesting as the namespace for all the testing. This helps
in the Doxygen documentation.

Bracket needs to go outside VexCL ifdef
Replaced by CppUnit version
@dmcdougall
Copy link
Contributor

I turned on all the optional libraries when Doxygen gets built and then put all the testing in a separate namespace (AntiochTesting). I was hoping the latter might help organize the documentation better, but it didn't help all that much, IMHO.

There's a 'namespace' list that Doxygen generates so you can look at just the classes in that namespace.

@dmcdougall
Copy link
Contributor

Looks good to me. Good work @pbauman.

pbauman added a commit that referenced this pull request Sep 9, 2015
Add CPPUnit Testing Framework
@pbauman pbauman merged commit e330197 into libantioch:master Sep 9, 2015
@pbauman pbauman deleted the cpp-unit branch September 9, 2015 12:43
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.

2 participants