put test cases inside the corresponding projects
imzhenyu opened this issue · 11 comments
Problem:
rDSN adopts the modular approach and many of the components are required to be a dynamic linked libraries (e.g., dsn.core.so, dsn.tools.common.so, dsn.tools.hpc.so, dsn.replication.stateful.so). The problem for dynamic linked libraries is that their internal data structures and symbols are not visible to the outside (on Windows, Linux does not do so so far but it is not good design by exposing the details). On the other hand, the test cases need to access the internal data structures and symbols for testing purpose, which requires linking against the static libraries (the current way we adopts now). This requires each module to be built into both static and dynamic libraries.
Solution:
We may move the test cases into the corresponding dynamic linked projects, so that we don't have to have two projects for each module? In fact, embedding the test cases into the projects where they are about to test helps us to write more test cases easily without setting up a new project, and we can also come up a new dsn_run_all_tests API for quick test for all modules (as long as they are loaded by dsn.core.so).
@qinzuoyan @shengofsun @xiaotz @ykwd @glglwty Since you guys wrote many of the test cases, what do you think of this?
Are there some cases that we need to use many modules to test a single feature?
@ykwd good question. There is indeed such requirement, which we call global checker (not using gtest). This requires access the internal data structures across multiple modules - this is not an issue on Linux (even they are not explicitly exposed - a BUG on Linux), but it is a problem on Windows (you cannot access symbols that are not exposed using dllexport on Windows). Fortunately, such global checkers requires not many information and we can ask developers to explicitly expose the required information (or we only do that on Linux).
To immagrate our current test code to this new framwork, little effort is needed for those local checkers(compared to global checkers) and much effort may be needed for global checkers. Hope that we have not many globle checkers requiring access the internal data.
@imzhenyu I'm not sure how to move the test cases in your way, but I don't think it's a good idea if you want to link the logic code and the test code together into a same loadable object. We may put the logic and test in the same folder, and produce two loadable objects(one for static/shared library, another for the test executable) by modifying the cmake files.
@shengofsun It is easy - simply putting the test files into the same folder (e.g., aio_provider.test.cpp is putting tougher with aio_provider.cpp), it's done. We can add default gtest linkage in dsn.cmake and a default core-provided dsn_run_unit_tests to run all test in this process. There are several benefits:
- easy to add test cases therefore people are more willing to do that (no need to write specific CMakeLists.txt and new folders)
- consistent way to run the tests, now we have multiple ways for each different tests - not enforced
- no need to change the scripts etc. (as all tests are through the same interface)
- no need to compile the same code twice (for loadable object and for testing)
There is one disadvantage though:
- the target binary becomes bigger and the test code are not used on deployment, which is not a big deal though since the code size is never an issue given the large memory we have.
Any other pros and cons in your mind?
There is another annoying issue due to the co-existence of both the static and dynamic linked libraries. Considering dsn.core.tests which we tried to use static linking for testing purpose, it loads dsn.tools.common.dll for tool registration. The latter actually loads dsn.core.dll as it is built atop dsn.core.dll. In the end, we have two versions of all symbols in core, one in exe and the other in dll, resulting problems such as the 'nativerun' factory is not found in dsn.core.tests because theyre registered in dsn.core.dll! This problem does not arise on Linux yet due to that Linux finds a symbol in the global space even though that the symbol is from a specific module during linking. This, however, is never a good practice (and may cause weird problems in the end, e.g., due to the change of the global search order). By enforcing one kind of linkage approach (dynamic only), we can eliminate these (potential) problems.
I think problems like this may result from our solution been unreasonable organized, and we should take efforts to refactor our modularity, not the test.
And unit test code should be directly linked with the logic source code, but not the static library or shared library. So compiling twice is inevitable.
And I think we should avoid larger target binary size, because smaller binary is friendly to both deployment and performance.
@imzhenyu As we will have a meeting this afternoon, let's talk about this face to face later :)
Yes, and I don't quite get what you said above - let's discuss face to face instead. See you soon~
It is done. Now for all the project we can have two kinds of test:
(1) gtest unit test
The test engine will search for a file named build-dir/test/project-name/gtest and run dsn.svchost config for each line in the gtest file which specifies a config file name under the same directory.
(2) custom test
The test engine will also search for build-dir/bin/project-name/test.sh (or .cmd on windows) and run that script accordingly.