search

FEniCS / dolfinx

Next generation FEniCS problem solving environment
https://fenicsproject.org
GNU Lesser General Public License v3.0
720 stars 177 forks source link

Separate unit tests from integration tests #1652

Closed jhale closed 3 years ago

jhale commented 3 years ago

Unit tests with reasonable runtimes are important, but there there is currently a lack of complete end-to-end tests in that test the variety of complicated and/or state-of-the-art numerical methods that can be implemented using the components of FEniCSx.

It would be good to add a suite of "integration tests" into DOLFINx with longer runtimes, that could help highlight any subtle bugs that do not show up with unit tests that typically test only isolated functionality. These integration tests could also serve as helpful hints for users where we do not want to provide a fully fledged demo. Due to the extended runtime, these tests could run twice a day.

These could be located in a new folder python/tests/integration.

Examples:

https://github.com/FEniCS/dolfinx/pull/1649 https://github.com/FEniCS/dolfinx/blob/main/python/test/unit/fem/test_fem_pipeline.py#L192 https://github.com/FEniCS/dolfinx/blob/main/python/test/unit/fem/test_fem_pipeline.py#L120

garth-wells commented 3 years ago

I'm not convinced that distinguishing 'short' and 'long' test has been successful or helpful in the past. The key is tests that stop bugs creeping into main in the first place. I'm not convinced that slow integration tests can pick up issues that can't be picked up by suitable, fast (unit) tests.

I think the objective needs to be testing that stops bugs getting in main. Testing that shows up a problem at an unknown point between multiple commits to main isn't very helpful - it can be hard to figure out what change(s) caused a problem changes cannot be easily reverted.

A preferable step, if necessary, would be running more tests on PRs. We are doing this now already, with the Intel compilers tested on PRs and not on 'plain' commits to a branch.

jhale commented 3 years ago

You're right, having a set of tests run twice a day on main is not a great idea.

"A preferable step, if necessary, would be running more tests on PRs. We are doing this now already, with the Intel compilers tested on PRs and not on 'plain' commits to a branch."

This could work if the number of tests becomes a problem.

For now I would propose that people just add more unit/integration tests and we'll see where we get to.