Drake: Poll: Where to put test files?

[david-german-tri]

Two comments follow, thumbs-up the one which you prefer. Suppose there are production files drake/foo/bar.h and drake/foo/bar.cc.

[david-german-tri]

drake/foo/bar.cc
drake/foo/test/bar_test.cc

[david-german-tri]

drake/foo/bar.cc
drake/foo/bar_test.cc

[jwnimmer-tri]

Explaining my vote:

In many projects, I prefer the former (a separate test directory) to help keep filecount per directory lower. It also facilities creating test libraries that live next to the tests, but are more clearly partitioned from production code. (So you have libcommon source files in e.g. common/*.cc, libcommon_test source files in common/test/*.cc, and test program source files also in common/test/*_test.cc but the _test.cc suffix distinguishes them. Then, you have e.g. libsystem_test code and/or other tests in system/test/ use libcommon_test, but libsystem is not allowed to use libcommon_test.)

However, I've always had a fixed directory tree depth (all libraries are laid out as top-level siblings, so mindepth=maxdepth=1 for production code, mindepth=maxdepth=2 for test code). In other words, there were directories foo and foo/test, bar and bar/test, etc. There was not foo/test and foo/bar/test and foo/bar/baz/test like we have with systems/test vs systems/plants/joints/test and etc., where the test subdirs are intermingled with sublibrary directories. If we keep the deep tree depth like we have now, I think we should stop using test as a separate subdir, to better group the tests with the code they are testing. If we flatten the tree to put all libraries at the top level, I'd vote to keep tests in subdirs (but happy to live with being outvoted).

[liangfok]

Since I believe the ideal software architecture that Drake should eventually adopt is component-based, I hope Dake's deep directory structure can eventually be simplified to be:

• component1/
• component1/test/
• component2/
• component2/test/
• ...

I don't see how component-based architectures prevent Drake being a glass box since the states that make it a glass box can be included with the messages that are transmitted between components, and ultimately with the outside world.

Maybe we should table this decision regarding location of unit tests until the more fundamental question of whether we adopt a component-based architecture is decided.

Related: https://github.com/RobotLocomotion/drake/issues/2152#issuecomment-214556160

[david-german-tri]

Explaining my vote: I far more often think "hey, I need to look at the tests to understand this file" than "hey, I need to look at all the tests for this directory". Therefore, it's useful to have the _test files lexicographically right next to the production code they test.

[david-german-tri]

@liangfok I don't follow. Are you saying component-based architecture requires tests to be in a separate subdirectory? Why would that be true?

[liangfok]

@david-german-tri: Looking up at @jwnimmer-tri's justification of his vote, he says:

In many projects, I prefer the former (a separate test directory) to help keep filecount per directory lower. ... However, I've always had a fixed directory tree depth ...

Basically, he voted to keep the unit tests in the same directory as the code they test because of the high directory tree depth that's currently in Drake. I was responding saying I think Drake should adopt a more component-oriented architecture and a corresponding directory structure that is wider but flatter. If we're able to do that, I believe @jwnimmer-tri will change his vote to keep the unit tests separate.

Stylistically, I think unit tests are not part of the component and thus should be kept physically separated from the code that actually implements the component. Placing them in a subdirectory would achieve this.

[amcastro-tri]

What about Matlab tests? should we have a matlab_test subdirectory?.
I like @liangfok's component based view. In the future the Matlab code will only be wrappers to C++ code. I think then that these wrappers should be in a completely different directory tree separate from the C++ source tree. However now we have them all in one single place....

[sherm1]

Stylistically, I think unit tests are not part of the component and thus should be kept physically separated from the code that actually implements the component. Placing them in a subdirectory would achieve this.

This is the strongest point for me. I don't like the apples-and-oranges mix of test code with the component code.

I would like a flatter directory structure but I don't feel strongly that it should have maxdepth=2 if there is some _meaningful_ substructure.

I think the "component architecture" discussion is confusing things here. I _think_ @liangfok is referring to a set of independent modules connected by message passing, like the ROS architecture (please correct me if I'm wrong, Liang.) That's not on the table here! Drake builds tightly-coupled libraries (for good reason in places), so I think the rest of us are using "component" in a less specific way to indicate one of those libraries.

[liangfok]

@sherm1: Yes, you are correct. Sorry for mucking up the discussion. It's just I feel adopting a more component-oriented approach will solve many of our problems and render many debates irrelevant, see, for example this new conflict that I just discovered.

[david-german-tri]

I have to say I don't get it (directories are not the same thing as build targets!), but it's pretty clear I am outvoted. I'll update the style documentation.

[liangfok]

IMHO, for conceptual clarity, the better the correlation between build targets and the physical co-location of files, the better.

[david-german-tri]

I guess that's the fundamental disagreement. I think it's common for source code to be conceptually related enough it should go in the same directory, but not mutually dependent such that it has to go in the same build target. (Tests are one example, but this is also a common pattern in production code. In Drake, I think we're likely to see it in /common especially.)

Anyway, not worth further resources to debate.

[liangfok]

Agreed. This is becoming a philosophical / religious difference of opinion. :blush: