machinekit
We need a (regression) test framework which actually works, runtests has to go
I'm loosing patience with this utterly naive approach to 'testing' which we inherited - this is the singlemost biggest time waster. This has to go.
The current suite of 'tests' is so full of race conditions and using naive programs and scripts that in many cases the failure of a runtest is basically meaningless - it usuallly just shows the author has not considered timing of parallel activities and how they can be properly interlocked.
Milestone proposal: deprecate runtests.
For each 'failure' found, rewrite the test with the new cython bindings once we have them, make sure the test actually says something, and disable the old one.
See https://github.com/machinekit/machinekit-cnc/issues/36 for full text, this is a copy header only.
I made a mistake in pull request #286 when rewriting test header-sanity: instead of HEADERS_DIRECTORY=$(readlink -f ../../../include) I put there HEADERS_DIRECTORY=$(readlink -f ../../include). When investigating what really happens when the HEADERS_DIRECTORY is pointing to non-valid path, I discovered that the find command correctly report an error and exits with 1. As the script is set -x, the whole execution eds there. That way I should error out and the test should fail - as the whole script has non-zero return code. ~~Problem is, the runtests program does not report this test as failed, but actually reports it as passed. I think this is a bug.~~
Also, the header-sanity tests doesn't really meet criteria for runtest (in my opinion) - it is not a run-time test but a build-time test. When one install the machinekit-hal-dev package and download source tree from git and then try running these tests, this one will always fail, because it needs the make to run beforehand. So the workflow of building the packages, installing them all (testing co-installability) and then running the tests on (pretty much) clean tree is broken.
Turns out, it is not problem with runtests, but actually bug in my own code. Apparently the find does not exit with no-zero return code when the directory doesn't exist. Odd, but a fact.
I made a mistake in pull request #286 when rewriting test
header-sanity: instead ofHEADERS_DIRECTORY=$(readlink -f ../../../include)I put thereHEADERS_DIRECTORY=$(readlink -f ../../include). When investigating what really happens when theHEADERS_DIRECTORYis pointing to non-valid path, I discovered that thefindcommand correctly report an error and exits with 1. As the script isset -x, the whole execution eds there. That way I should error out and the test should fail - as the whole script has non-zero return code. ~Problem is, theruntestsprogram does not report this test as failed, but actually reports it as passed. I think this is a bug.~Also, the
header-sanitytests doesn't really meet criteria forruntest(in my opinion) - it is not arun-time testbut abuild-time test. When one install themachinekit-hal-devpackage and download source tree from git and then try running these tests, this one will always fail, because it needs themaketo run beforehand. So the workflow of building the packages, installing them all (testing co-installability) and then running the tests on (pretty much) clean tree is broken.Turns out, it is not problem with
runtests, but actually bug in my own code. Apparently thefinddoes not exit with no-zero return code when the directory doesn't exist. Odd, but a fact.
Interesting distinction between "run-time" and "build-time" test. I agree that this and all tests should pass in isolation, even removed from the git repo, when packages are installed. That was (in spirit, anyway) a goal of recent PRs where runtests was installed to /usr/bin; I expect this won't be the only problem test we find.
Anyway, here's how I dealt with this problem on the EMC side. This should work for the HAL side as well. External sources building against HAL can run pkg-config --cflags machinekit-hal, which should return -I<header_dir>, and should be correct both for system-installed headers and in-tree headers for RIP builds (if rip-environment has been sourced, of course).
...and as soon as I clicked the "Comment" button, I see why this addresses a problem you didn't ask about (even though it may come useful by the time the test is working again).
Since the system-installed header directory is /usr/include/machinekit, that test could be made to work in various ways, for example:
- Isolate
$HEADERSfrompkg-config --cflags machinekit-hal(dirty and prone to failure) - Add an additional variable to
machinekit-hal.pclikepkg-config --variable=header-path machinekit-hal(clunky with no use outside this test, but reliable) - Template a file
tests/build/header-sanity/headers_path.incontaining the header directory path@HAL_INCLUDE_DIR@(cleaner, but still requires minimally configuring the source with./autogen.sh && ./configure)
Interesting distinction between "run-time" and "build-time" test.
Some old gun at testing would have a field day with my naïve terminology, I am sure. But for my purposes it is adequate, I think. Will see in 20 years how fine I am with it, then.
I agree that this and all tests should pass in isolation, even removed from the git repo, when packages are installed. That was (in spirit, anyway) a goal of recent PRs where runtests was installed to /usr/bin; I expect this won't be the only problem test we find.
Probably not, I had some other problem with different test (which name now I cannot remember) when installing all available Machinekit packages on clean system, cloning the git repository and running tests.
Anyway, here's how I dealt with this problem on the EMC side. This should work for the HAL side as well. External sources building against HAL can run pkg-config --cflags machinekit-hal, which should return -I<header_dir>, and should be correct both for system-installed headers and in-tree headers for RIP builds (if rip-environment has been sourced, of course).
That's one way how to do it. But you will slightly (but fundamentally, if that's possible combination) change the meaning the test. Because so far it was testing the headers used for compiling Machinekit - if they don't contain some brain-dead code which would not cause compilation error, however. But now, you will be testing the package machinekit-hal-dev and what headers it installs.
Since the system-installed header directory is /usr/include/machinekit, that test could be made to work in various ways, for example:
Frankly, I have no idea which solution to choose. Even after few days.
I have a feeling from runtests that it is trying to encompass too many tests stages. Stages, which by today standards are solved by multitude of solutions. One of the reasons why I liked inclusion (first small one, but still one) of unit tests.
(I guess the whole thread was about this problem.)
It made me think if some pre-made solution doesn't exist for this problem. Because if tests will stop being tightly connected to the installation, then you will have to somehow direct which ones to run, what dependencies they have (the header-sanity one needs gcc and g++) and what output they should have based on the state.
I have been looking at documentation of Robot Framework, but so far cannot tell if it would be beneficial.