navit-gps / navit

The open source (GPL v2) turn-by-turn navigation software for many OS
http://www.navit-project.org
Other
555 stars 175 forks source link

Unit testing in Navit? #561

Open aerostitch opened 6 years ago

aerostitch commented 6 years ago

Hi guys,

I wanted to add some unit testing in Navit, that would allow people like me to better test their changes. Would you be ok with that? If so what unit test framework would you be comfortable to go with?

Thanks for your help, Joseph

jandegr commented 6 years ago

Hi,

Anything related to CI and Android should also work on Debian, that's what their dedicated android containers are built on.

thx

aerostitch commented 6 years ago

The frameworks I was thinking about are:

metalstrolch commented 6 years ago

Personally I don't care about which unit test framework to use, as long as it adds no exotic dependency on non unit test build. Navit is quite often cross compiled, and there such dependencies are hard to fullfill.

Additionally, one should think about suitable granularity to test. What are our "units". Can be hard on legacy code already existing.

I think a first step could be better documenting the plugin interfaces (like "graphics" or "vehicle") and try to craft proper test functions for this. Would at least ease up creating proper plugins. And I would be really curious if all plugin interfaces are still known. I myself don't for example know all callback function's properties for the graphics interface. Even after I ported a graphics plugin.

aerostitch commented 6 years ago

I was thinking unit = function. We could also do integration testing which would be at plugin level. Agreed for the documentation. I think we should start with a "create a navit plugin 101" I've created a "Documentation" project on: https://github.com/navit-gps/navit/projects/5 Feel free to add more ideas in it.

metalstrolch commented 6 years ago

Disagree in unit=function. One of the most common mistakes in doing unit tests. Will create LOTS of unit test code with not much benefit. Will even hinder improvements. as nobody likes writing test code.

Usually the unit to be tested should be tied around architectural defined interfaces. So if a function forms a publicly visible interface (like the plugin functions do) then this is what you want to test. Otherwise people will start writing spaghetti functions just for not having to write another unit test...

aerostitch commented 6 years ago

Sounds good. Do you have specific frameworks in mind that would help around that or a set of rules we should try to stick to when writing those kind of tests @metalstrolch ?

metalstrolch commented 6 years ago

Not really. First thing to do would be to document architecture to find public interfaces. That's why I propose to start with the plugins, as those interfaces are at least easy to identify and would allow to check if all plugins of same kind behave equal. And this would allow to introduce the choosen framework first. Otherwise functions present in module public headers are a good starting point too. It's about defining which function somebody using the component should actually use. This makes the components interface.

Not knowing the core functionality well enough, but I think there things like map data handling, routing algorithm and such could be identified to add another testworthy in-core interfaces.

This is unfortunately good old manual work that cannot be automated. This is why it is that hard for legacy code where nobody documented / enforced architecture.

aerostitch commented 6 years ago

Question that might sound a bit stupid, but when you are updating/fixing functions, how do you test that the outcome works as expected? Do you compile everything or you make a separate main() that calls your function and makes sure that the input and the output are what you expected? (Just wondering as I did face the issue and it would have probably have been easier for me if I had a documented way to make sure a small change works aside from having to recompile everything, install on different platforms and manually test the outcome)

metalstrolch commented 6 years ago

Unit tests are usually always run by their own main. Even if your unit test framework may hide that from you. You usually even want to compile your code + mock ups for required dependencies + the actual unit test function on a different arch. Then your build machine can natively run them. But this requires the code at least being structured in a way that your to be tested code can be built separately, by for example moving hardware / arch dependent things in separate input files. This may currently be not always the case.

You can only check changes on the granularity that you choose. For example: If you have a test checking the interface of a plugin, this test will of course only detect defects influencing the plugins behaviour against that. But you won't need an automated test to check some internal few line function. It's a matter of finding the correct balance between the number of tests against test depth. Wanting to have automated tests at least enforces one thing: One needs to properly define the interface to be tested.

Simple example: If the lines of code of your tests equals the code in test, what is more probable: Having a defect in the code or having a defect in the tests.

aerostitch commented 6 years ago

How about we allow both and strongly recommend unit testing at the public interface level?

Personally I don't have a strong experience in C like you guys do and would be glad to have an easy way to quickly test a change on the function-level on the different platforms we build on? When I push a change I push a corresponding test (that i can remove later on if you prefer).

We would just need a smart way to allow both (at least as long as we don't have all the public interfaces properly tested).

charlescurley commented 6 years ago

We could also use unit testing for discrete chunks of the code. I am seeing a problem with the interface to GPSD. So I started a bisect like a good little git user. Well, very close to the last step, I got another error, one I don't believe is related to the bug I am trying to track down. A good unit test for the gpsd interface code might have caught the bug I am chasing early on.

pgrandin commented 6 years ago

@charlescurley do you have a tool/framework to recommend for that?

charlescurley commented 6 years ago

On Thu, 27 Sep 2018 18:10:09 -0700 Pierre GRANDIN notifications@github.com wrote:

@charlescurley do you have a tool/framework to recommend for that?

No, I do not. Sorry.

-- "When we talk of civilization, we are too apt to limit the meaning of the word to its mere embellishments, such as arts and sciences; but the true distinction between it and barbarism is, that the one presents a state of society under the protection of just and well-administered law, and the other is left to the chance government of brute force."

lains commented 6 years ago

cpputest is a framework I have been using in the past. You can decide which unit boundary you're testing, so you can unit test a function or functional test a whole module, or even the whole program if you can manage to send stimuli and receive feedback in a programmatic manner. It works pretty well, even though I had trouble with memory leak detection and sometimes getting it to work (link with the code) takes quite a while, and the debugging logs were not helping much when a test was failing (but maybe I did it wrong). Google Test is also well known, it seems more oriented to C++ but can probably test pure-C code without much trouble. The advantage is that it is C/C++ coding so C/C++ developers can easily develop unit tests, but this then means knowing C/C++. And of course, automated unit test like this can be run to validate changes (pull requests).

aerostitch commented 4 years ago

I started to play with CUnit today for testing corner cases of a specific function that I got out of the code of Navit (it helped me a lot validate my change and be more confident about what the function was actually doing which helps when doing the documentation): https://gist.github.com/aerostitch/faa106d7534416c4dd9dc7e57dbfd209 Based on what @jkoan has seen it seems pretty straightforward to insert into cmake: http://nuno-sousa-corner.blogspot.com/2009/10/back-to-c-cmake-and-cunit.html?m=1

I'll try the same test function with the 2 other frameworks @lains was talking about.

In the meantime I was wondering if you had preferences in terms of file naming and location convention to take (should it be in a subfolder called "tests" at the root or in a subfolder called "tests" in each directory or directly along with the actual code file?).

@jkoan was proposing:

one tests folder with files named test_plugintype_component.c so for example the full path could be navit/tests/test_gui_gtk.c

Is everybody onboard with it?

aerostitch commented 4 years ago

Also if you're curious I've added an example of the 3 framework we were talking about:

[----------] Global test environment tear-down [==========] 1 test from 1 test suite ran. (0 ms total) [ PASSED ] 0 tests. [ FAILED ] 1 test, listed below: [ FAILED ] SearchTestSuite.search_fix_spaces

aerostitch commented 4 years ago

I'm going to close this as there is some work for it started in https://github.com/navit-gps/navit/tree/add_tests If there's any traction for it we can reopen #938 or continue on the add_tests branch.

jkoan commented 1 year ago

Reopening as I think it makes sense to write unit tests and #1215 is a perfect example of what can be tested. Other things might be more complicated to test but it will be worth one we get started.