Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

we need a regression test framework which actually works. runtests has to go. #36

Open
ArcEye opened this issue Aug 2, 2018 · 21 comments
Labels
bug Something isn't working Cross-Stacks Issue Applies to both machinekit-cc and machinekit-hal

Comments

@ArcEye
Copy link
Collaborator

ArcEye commented Aug 2, 2018

Issue by mhaberler
Sun Jun 15 06:57:00 2014
Originally opened as machinekit/machinekit#220


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.

@ArcEye ArcEye added bug Something isn't working problem labels Aug 2, 2018
@ArcEye
Copy link
Collaborator Author

ArcEye commented Aug 2, 2018

Comment by zultron
Sat Oct 11 06:20:17 2014


So, Michael's Machinetalk merge included some nosetests. PR #332 isn't ready for merge yet, but intends to fix some minor problems, and then hopefully start running it in the buildbot.

See #105, just closed, for griping about the old system.

@ArcEye
Copy link
Collaborator Author

ArcEye commented Aug 2, 2018

Comment by mhaberler
Thu Jul 9 05:33:36 2015


and we need a way to automatically regression-test supplied configurations against API changes.

Currently there is no such thing, which creates a lot of work post-fallout.

For the most part it would be sufficient to start configs without actually starting a UI, and see if it 'comes up' (i.e. does not fail)

The only configs which would not work that way are ones which rely on a UI creating pins, like the ones using gladevcp and POSTGUI_HALFILE = feature, but those could be automatically excluded.

Any takers?

@ArcEye
Copy link
Collaborator Author

ArcEye commented Aug 2, 2018

Comment by machinekoder
Thu Jul 9 06:27:19 2015


All configs that depend on specific hardware cannot be tested this way. One could however use the Python based HAL configs to add a test flag for excluding hardware specific functions.

@ArcEye
Copy link
Collaborator Author

ArcEye commented Aug 2, 2018

Comment by mhaberler
Thu Jul 9 06:31:54 2015


@Strahlex that is a pretty good idea!

what I can imagine - add to the exit handler in the cython hal module to check for uniqueness of names.

something similar can be added to halcmd to check on exit.

re hardware - regression testing the sim configs is already a start.

@ArcEye
Copy link
Collaborator Author

ArcEye commented Aug 2, 2018

Comment by machinekoder
Thu Jul 9 06:50:23 2015


We could also add proper per-component runtests or simplifying/unifying the process of creating one. Shouldn't be too hard using a HAL Cython. This would require a way of triggering HAL functions manually from HAL Cython with custom arguments (period for example).

@ArcEye
Copy link
Collaborator Author

ArcEye commented Aug 2, 2018

Comment by machinekoder
Thu Jul 9 06:59:56 2015


Just updated machinekit/machinekit#391 Qt has a high sophisticated sanity checking system. Maybe we could use parts of it to prevent common problem. Regression tests are also triggered and connected to PRs. They use Gerrit as Review/Merge tool.

@ArcEye
Copy link
Collaborator Author

ArcEye commented Aug 2, 2018

Comment by machinekoder
Thu Jul 9 07:13:48 2015


Additionally the testing framework should be language independent so we can use it for reporting Python tests as well C or Shell tests. Not sure if something like this exists.

@ArcEye
Copy link
Collaborator Author

ArcEye commented Aug 2, 2018

Comment by machinekoder
Thu Jul 9 08:21:00 2015


Btw. another way to test configs that require specific hardware without needing the hardware is using stub HAL components that have the same pin names and types but no or simulated functionality.

@ArcEye
Copy link
Collaborator Author

ArcEye commented Aug 2, 2018

Comment by mhaberler
Tue Aug 25 09:08:47 2015


I think we need to grind this cat per-language, as I dont see a regression test package which covers all of them. Current thinking:

all of those are debian packaged: apt-get install python-nose check libgtest-dev

so my current proposal would be: organize per-language under src/regressions:

  • regressions/check - C tests using check
  • regressions/nosetests - moved from nosetests
  • regressions/gtest - C++ tests using googletest
  • regressions/runtests - the legacy test arcana

configure.ac will detect prerequisite packages and build as needed

a 'make check' target runs all of the above if built

a hello-world example for using check is here: mhaberler/machinekit@ec14144

example output of a failed check test:

Running suite(s): HAL
0%: Checks: 1, Failures: 1, Errors: 0
regressions/check-tests/check_hal.c:7:F:Core:test_helloworld:0: Assertion '0==1' failed: 0==0, 1==1
Makefile:1799: recipe for target 'check' failed
make: *** [check] Error 1

@ArcEye
Copy link
Collaborator Author

ArcEye commented Aug 2, 2018

Comment by zultron
Wed Aug 26 02:15:43 2015


On 08/25/2015 04:08 AM, Michael Haberler wrote:

I think we need to grind this cat per-language, as I dont see a
regression test package which covers all of them. Current thinking:

all of those are debian packaged: |apt-get install python-nose check
libgtest-dev|

When I refactored the nosetests as part of the 'master daemon' project,
I simply wrote Cython bindings for the C stuff, and then wrote more nose
tests. There's another possibility.

@ArcEye
Copy link
Collaborator Author

ArcEye commented Aug 2, 2018

Comment by mhaberler
Tue Sep 15 06:33:55 2015


I have been using check with success, simple to understand and does the job; other than Cython wrappers it can be used for timing critical code, and multithreaded tests (of which I have quite a few now)

For C++ I found a less filling alternative to gtest, catch - just a single header

I'll give that a spin

@ArcEye
Copy link
Collaborator Author

ArcEye commented Aug 2, 2018

Comment by yishinli
Thu Oct 1 05:49:17 2015


Hi Michael,

What's your most up-to-date branch for regression tests?

-Yishin

@ArcEye
Copy link
Collaborator Author

ArcEye commented Aug 2, 2018

Comment by mhaberler
Thu Oct 1 07:57:06 2015


Hi Yishin,

I have nothing mergable at the moment, but I have been working with Mick since several months on the SMP-safe HAL branch, and that contains several tests using check - have a look under src/regressions/check in https://github.com/mhaberler/machinekit/tree/disentangle

I have imported the catch header, but not used that yet for any actual regression tests

Assuming you are looking for C - my recommendation would be to install the check package and base your regression tests on that; maybe import the minimum from regressions/check to get going and we'll look into merging that in once the SMP-safe HAL branch is merged

For Python-based tests, see nosetests/*py and tests/nosetests - you need the python-nose package for that to work

@ArcEye
Copy link
Collaborator Author

ArcEye commented Aug 2, 2018

Comment by yishinli
Thu Oct 1 09:48:48 2015


Trying to run "make check", and got error message:

:~/proj/machinekit.mah/src(disentangle)$ make check
... ...
Compiling regressions/check-tests/check-util.c
regressions/check-tests/check-util.c:11:19: fatal error: ck_pr.h: No such file or directory
compilation terminated.
make: *** [objects/regressions/check-tests/check-util.o] Error 1

It seems like I need to install Concurrency-Kit, right?
Which version of Concurrency-Kit should I install?

-Yishin

@ArcEye
Copy link
Collaborator Author

ArcEye commented Aug 2, 2018

Comment by mhaberler
Thu Oct 1 11:27:30 2015


please do not base any work on this branch, and do not assume this will be merged as is - it is not ready, so there's no point in going for a build, and I do not want that branch to start creeping out early and setting any expectations

please see the docs at http://check.sourceforge.net/

just take regressions/check_tests as an example to build your own, and take the suite files as examples to read and learn from

take regressions/check_tests/check_tests.c and the Submakefile, plus the change in src/Makefile, and create your own test program from it; delete all the *-suite.h includes and the srunner_add_suite() calls which refer to the tests in the *suite includes

@ArcEye
Copy link
Collaborator Author

ArcEye commented Aug 2, 2018

Comment by yishinli
Fri Dec 25 22:18:13 2015


After installing ConcurrencyKit, I could compile the "disentangle" branch.

sudo apt-get install libck-dev

However, while running the "check_test", I got the following error message:

./check_tests 
ERROR: could not connect to HAL

This error came from hal_setup(), where (hal_data == NULL). How to dig why the hal_init() not initializing correctly?

void hal_setup(void)
{
    //    printf("number of cores=%d\n", cores);
    comp_id = hal_init("testme");
    if (hal_data == NULL) {
    fprintf(stderr, "ERROR: could not connect to HAL\n");
    exit(1);
    }
    hal_ready(comp_id);
}

@ArcEye
Copy link
Collaborator Author

ArcEye commented Aug 2, 2018

Comment by mhaberler
Sat Dec 26 08:23:39 2015


well you need to do a realtime start before running this test

hal_data being null means the HAL shared memory segment cannot be attached to, and that is created by rtapi_app

@ArcEye
Copy link
Collaborator Author

ArcEye commented Aug 2, 2018

Comment by yishinli
Sat Dec 26 10:53:43 2015


@mhaberler,

Thanks, it works!

100%: Checks: 14, Failures: 0, Errors: 0
wall time - all tests: 2108.583431 ms

In our testing environment, the zeroconf_service_withdraw() may occasionally being called; a SIGTERM is triggered to stop haltalk.

I'm thinking of writing unit test case to trigger zeroconf_service_withdraw().
Would you please give some advices?

@ArcEye
Copy link
Collaborator Author

ArcEye commented Aug 2, 2018

Comment by mhaberler
Sat Dec 26 11:47:03 2015


@yishinli - re haltalk:

are you suggesting haltalk crashes? I do not understand what could cause "zeroconf_service_withdraw() may occasionally being called" - it is only called after event loop termination?

I did notice sometimes haltalk fails to terminate, is that what you are referring to?

@ArcEye
Copy link
Collaborator Author

ArcEye commented Aug 2, 2018

Comment by yishinli
Sat Dec 26 13:00:54 2015


It seems like a haltalk crash. But, there's no segmentation fault message. The only clue I got is SIGTERM is received by haltalk. Also, it happens randomly from 20minutes to more than 2hours.

I am trying to gather more meaningful log message with DEBUG=5. Any suggestions?

@ArcEye
Copy link
Collaborator Author

ArcEye commented Aug 2, 2018

Comment by mhaberler
Sat Dec 26 14:27:56 2015


@yishinli - if this is master, please open a new issue, this is simply the wrong issue
if this is mhaberler/disentangle, I cannot help - this is not ready and you should avoid it until it is

that said:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Cross-Stacks Issue Applies to both machinekit-cc and machinekit-hal
Projects
None yet
Development

No branches or pull requests

1 participant