shogun-toolbox / shogun

Shōgun
http://shogun-toolbox.org
BSD 3-Clause "New" or "Revised" License
3.03k stars 1.04k forks source link

Refactor GP unit tests to use data fixtures #3947

Closed karlnapf closed 7 years ago

karlnapf commented 7 years ago

A lot of the GP unit tests are defining the same data in multiple test cases, resulting in huge code files with large redundant parts.

This task is to use the fixture concept to pull data generation to a class that the tests then extend.

There are some tests in Shogun that are doing this (right @vigsterkr ?). It is a nice task to get familiar with fixtures, googletest and Shogun in general

Sahil333 commented 7 years ago

@vigsterkr can you give reference to those tests?

karlnapf commented 7 years ago

grep for GaussianProcess in shogun/tests/unit

durovo commented 7 years ago

Hello, I am trying to work on this issue as no one seems to have worked on it for quite some time. I have identified these two files that have GP unit tests: classifier/GaussianProcessClassification_unittest.cc regression/GaussianProcessRegression_unittest.cc I also went through some other shogun unit tests where fixtures have been used such as FisherLDA_unittest.cc, BinaryLabels_unittest.cc, and RandomForest_unittest.cc.

In the file GaussianProcessClassification_unittest.cc test cases GaussianProcessClassificationUsingSingleLaplaceWithLBFGS, GaussianProcessClassificationUsingKLCovariance, GaussianProcessClassificationUsingKLCholesky, etc. define and use the same data. Since the test case names are different in these, they cannot use the same test fixture class. Would it be fine to create a base fixture class and create several other classes with appropriate names for each test case that inherit from this base class?

karlnapf commented 7 years ago

Hi @durovo and welcome! Great that you want to work on this

That would be ok. In fact, anything that reduces the bloated code.

Let me know if you have any questions

durovo commented 7 years ago

Hello @karlnapf , I have refactored GaussianProcessClassification_unittest.cc. However in GaussianProcessRegression_unittest.cc, the same data is almost never used more than twice in the nine tests. Also, since we can have only one fixture class for each TestName, I cannot think of any good way of refactoring it.

karlnapf commented 7 years ago

Closed in #3990