Refactoring #1430

Consider moving FormFactor unit tests to a single functional test

Added by pospelov over 3 years ago. Updated over 3 years ago.

Status:ResolvedStart date:12 May 2016
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:-
Target version:-

Description

I think that tests of form factor behaviour near singularity and along symmetry axes belongs to the functional tests, and not to unit testing.

Here are some supporting arguments

  • Conceptually it is not test of a unit, but rather the model behind the whole object
  • Amount of such unit tests (more than 200k of tests!)
  • Execution time (more than 10sec)
  • The necessity to patch google-test gives a hint that there is a misuse of the library
  • Aesthetic of the resulting output
    • [ RUN] OK is better than just [——]
    • How one can make a screenshot for a presentation showing last lines of unit test output with total amount of tests?
  • Functional test machinery is able to run a custom test
    • Just any additional application as in FunctionalTest/TestCore/PolDWBAMagCylinder

Additionally, I find the new directory structure for unit test confusing

One letter in directory name (0, 1, 2, P, Q, S ?)
Naming of command files _TestCore0, _TestCore2 etc.

I personally have a slight preference for a single directory.
The minor advantage is that everything is in one place; our build server reports “321 unit tests passed” and not “26 unit tests” passed. Aesthetically, again, I’m happy to see one single table with testing results of just compiled library, but not 10 small tables here and there.

But if we want to split to several directories, why not to have same “partitioning” as in Core library itself (Algorithms, FormFactors, InputOutput, etc…). At least it will be easier to find what belongs to what.

History

#1 Updated by wuttke over 3 years ago

These are two different issues:
  • Move extensive ff tests from Unit to Functional?
  • Don't split Core/Unit tests into subdirectories, or split them differently?

The nexus between these two is: Splitting unit tests allows parallel execution. Parallel execution is desirable anyway, but particularly for the new, extensive ff tests.

#2 Updated by pospelov over 3 years ago

I put two issues in one, that's true. The major issue is to move Unit to Functional.

#3 Updated by wuttke over 3 years ago

Nothing in googletest says it is limited to unit tests, let alone to unit tests in the most narrow sense. Why do we actually have a functional test framework of our own?

#4 Updated by wuttke over 3 years ago

[ RUN] OK is better than just [——]

Agree. Implemented in 91d8e8f5b0.

#5 Updated by pospelov over 3 years ago

My previous reply disappeared since you commented same record I was commenting.

Why do we actually have a functional test framework of our own?

It's now we, it's CMake/CTest. Designed for complicated things after the build: configuring, testing, memory checking, even submitting results of test to CDash.

Of course, one can use Google Unit Test for functional testing too (in all three core/gui/python domains ;).

But the main point is about Unit <--> Functional testing. These are two different things. Heavy staff is going to functional testing (make check), that's my opinion.

#6 Updated by herck over 3 years ago

I also agree that these tests conceptually do not belong to unit tests. It also obscures unit test coverage (having 200K tests for a few 100 classes). I also agree on moving the directory discussion to a separate issue.

#7 Updated by wuttke over 3 years ago

The number 200k comes from arbitrary choices and can be changed if time is a concern.

The tests are concerned with numeric correctness of one specific method, evaluate_for_q. In my understanding this is prototypical unit testing. The more so since you say: functional testing is »for complicated things after the build«.

Even without the new ff tests, just for the few 100 unit tests we had before, the output from the googletest default listener was overly verbous. The output generated now (91d8e8f5b) is fully sufficient. In case of failure, it still gives full information on where it happened.

#8 Updated by wuttke over 3 years ago

  • Status changed from Rfc to Resolved

The main concern will be resolved per issue #1433.

Renaming of subdirectories will follow later, when Core/ is restructured.

Also available in: Atom PDF