[all] Refactor SofaTest to cut dependencies

[damienmarchal]

This is a variation of the proposal done by @fjourdes in #455 where I allow me a bit more refactoring.

Here is what the PR does:

• implement basic test class in sofa/helper/testing/ BaseTest & NumericTest so that all framework tests can use them.
• in SofaSimulationGraph/testing add a BaseSimulationTest class that implement facilities for tests that needs scene manipulation & loading.
• udpates SofaTest to use the previously defined classes but still behaving as they used to in the past.
• updates the framework_test to use BaseTest or NumericTest in place of Sofa_test or ::testing
• moves the tests in framework to framework_extra when they are manipulating a simulation (thus requires BaseSimulationTest instead of BaseTest)
• updates the tests in simulation_test so that they don't use SofaTest anymore.
• moves SimpleApi from SceneCreator to SofaSimulationGraph & fix a bug.
• creates a plugin in a the application/package directory so that in our tests we can load all the common sofa component with a single RequirePlugin("SofaAllCommonComponents")

The consequences are:

• SofaTest is no more needed in framework & simulation
• we can now progressively make the other tests to rely on BaseTest, BaseSimulation and SimpleAPI that are dependency free instead of SofaTest/SceneCreator.

Feel free to provide feedback/comment.

---

This PR:

• builds with SUCCESS for all platforms on the CI.
• does not generate new warnings.
• does not generate new unit test failures.
• does not generate new scene test failures.
• does not break API compatibility.
• is more than 1 week old (or has fast-merge label).

Reviewers will merge only if all these checks are true.

[damienmarchal]

One possible next-step could be to connect the BaseSimulationTest with the simpleapi work discussed with @maxime-tournier. So that we would have a fully loosed coupling equivalent to scenecreator and sofatest with good looking syntax.

[fjourdes]

I believe whatever the approach taken it should allow to keep things pretty clear between

• unit tests, ie tests that hardly depend on initializing a scene. In these tests you hardly need a concrete instance of node or simulation. If you need this it may be nice to have a class to derive from which does it for you, but then again it should stay optional, because you should really think again about how you designed your component if you need a full fledged simulation to actually make some basic testing on your component.
  No matter what, these tests should be executed at each build (even for devs) and must pass. Failing in this kind of tests must be equal to a compilation error.
  In that respect there is something that I am reluctant to allow for this type of tests is a lose coupling approach where you instantiate components using keys from the object factory. You may end up writing tests that in fact do not compile from a scratch build because this allows you to not take care about the dependencies between the libs. That is to say a test may instantiate a component that is not available yet, because it belongs to a library which is deeper in the build dependency tree at the time the unit test is built and executed during the compilation.

• Functional tests, that require a working scene with probably more components than the one actually tested. These tests can be longer and therefore should not be mandatory during the compilation.
  They will be run only in ci machines, but can be also easily executed by any developers on its machine mostly to debug what is the nature of a new regression in these kind of tests.
  From our experience at InSimo these kinds of tests regress quite often during the development of a new feature, because you make some optimisation in the code, and you get some epsilon difference, or because you change the approach entirely and the scene is no longer relevant for you and should be changed. So you probably do not want too many of these functional tests, but there should be enough so that it is representative of the overall kind of simulations you want to support.
  But anyway it is still nice to have a record of the regressions that occur during the overall development, because you can then document better when you actually introduced changes in the code that are significant, which certainly is hard to track by unit tests only, unless you have a coverage of 100%, which is a target that in my opinion is a waste of energy to aim at, provided you have some functional tests also.

[damienmarchal]

Hello François (@fjourdes),

Thank for the long and interesting reply.

Your last comment rise an interesting question related to differentiating unittest to functionnal tests.
Currently in Sofa we don't make any distinction between unit test and functionnal test but my thinking is that in general utility classes have unit tests while components have functional tests . Making the distinction would be indeed better so the question is how to get there from where we are a now.

Here is a proposal.
In the current PR there is a BaseTest and BaseSimulationTest, this could actually be transformed without too much effort into UnitTest and FunctionalTest. The tests inheriting from UnitTest shouldn't use the factory in any way and shouldn't have external dependencies. The test in-heriting from FunctionalTest should use the factory and can make the assumption that specific plugins are loaded. We could event go one step further and emphasizing things by having two set of file, ones named MyClass_unittest.cpp and MyComponent_functionaltest.cpp and in our CMakeLists.txt compile the two set of tests separately with different build targets.

This would make things very clear, be compatible with your workflow where unit test are considered as compilation failure, be compatible with our workflow where functional tests are used because they allow to get decent code coverage and last point as it would use the loose coupling approach it would totally cut the dependency at compile time to SofaTest.

Small note, some Functional tests may rely on components implemented in plugins. I have not clear view if we should disable the test if the plugin is not there or if we should make it fails or if we should "disable" it at runtime.

EDIT: changes some sentences.

[bcarrez]

From what I understand, we have 2 different topics here.

1. cut dependencies to SofaTest, which I totally approve. This point reaches consensus I think. I don't see anything that can go against the merge of this PR in this topic

2. split unit tests and functionnal tests
   This is a different topic and should be discussed in a separate issue. Anyway my 2 cents about this:

• obviously this is a good idea. I cannot push forward enough this idea.
• just keep in mind that these unit-tests should remain FAST to run, since they will be run at each build. (ie. the filemonitor tests, for example, should be refactored to spend less than 10 seconds to run otherwise they will soon become a pain)

To resume my point of view:

1. merge this PR first
2. communicate around the fact that tests should not rely on SofaTest anymore (when possible) ==> why not log a warning or "deprecated" message when using SofaTest utilities ?
3. progressively cut unit tests from SofaTest, on-the-go

In parallel, open the "always run unit-tests" discussion on a separate topic.

[guparan]

My change is not a regression. Builds are now failing when unit tests crash for an unexpected reason (not due to the test itself).
Sorry to kind of break your PR @damienmarchal :-/

[damienmarchal]

@guparan No problem guillaume and you are totally right with that.
I spend the whole day yesterday to narrow the problem but cannot find it yet.

[damienmarchal]

@guparan & @fredroy
I think I fixed the test failure problem on centos & ubuntu by removing the
add_target_library( gtest ) in Sofa_test.
Don't ask me why it remove the failure at release I don't know ;)

[damienmarchal]

[ci-build][build-scenes]

[damienmarchal]

[ci-build][with-scene-tests]

[damienmarchal]

Maxime (@maxime-tournier)
I would appreciate to have your feedback on this PR.

[maxime-tournier]

Hi @damienmarchal, will look into it ASAP, but I don't have much time these days unfortunately :-/

[damienmarchal]

@maxime-tournier I would alreay be happy with a feedback on the approach taken to cut the depdencies. I'm asking because I know the topic interests you.

[damienmarchal]

@hugtalbot , @guparan and everyone else
This PR

• have discussed with as much as possible persons,
• and is now one month old,
• succeed on all Tests on the CI since several days

I suggest to have a last look at it & merge it rapidly that we can start working on top of it to improve it and to remove all the dependencies to SofaTest.

[hugtalbot]

@damienmarchal could you explain why this folder has been created ?

[hugtalbot]

is that for the unit tests?

[damienmarchal]

Hello Hugo,

Sure,

The goal of the PR is to cut the dependency between tests & core+modules. My problem was that modules are not plugins which was annoying to loose the dependency graph. So I decided to make plugin containing only the modules initialization so when you load the plugin the underlying modules get initialized properly. This is what packages/SofaAllCommonComponent is doing.

I created the folder packages to put this SofaAllCommonComponent to differentiate this from other real plugins that actually contains code.

Now, putting that in perspective I think this should and will be remove while we are moving toward the full pluginization and sofaNG re-organization.