GUACAMOLE-662: Fix handling of unit tests within build.

[mike-jumper]

This change corrects the way unit tests are handled within the guacamole-server build, logging the correct number of tests/passes/failures and failing the build overall if at least one test fails.

This required:

• Removing a ton of boilerplate C test runners, replacing them with generated code (see util/generate-test-runner.pl and README-unit-testing.md within these changes)
• Refactoring existing tests to leverage util/generate-test-runner.pl, taking the opportunity to clean up and reorganize accordingly.
• Fixing the way several of these tests fork() - parent/child logic was switched, resulting in race conditions in logging of test results.

The new util/generate-test-runner.pl script allows unit tests to be written as simple function declarations following conventions documented in README-unit-testing.md. So long as those conventions are followed, only the test functions themselves are required, and the necessary boilerplate to actually run those functions, collect results, etc. is generated automatically.

NOTE: Merging these changes will likely result in the build failing within CI when make check is run. This is actually a Good Thing, as the build really should have been failing all this time due to GUACAMOLE-510.

[mike-jumper]

For reference, the incorrect test handling of the current build produces the following erroneous report:

[mjumper@dev-mjumper guacamole-server]$ make check
...
PASS: test_libguac
============================================================================
Testsuite summary for guacamole-server 1.0.0
============================================================================
# TOTAL: 1
# PASS:  1
# SKIP:  0
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0
============================================================================
...
make[1]: Leaving directory '/home/mjumper/apache/guacamole/guacamole-server'
[mjumper@dev-mjumper guacamole-server]$ 

There is actually more than one test, and one of those tests (the nested socket test) is actually failing. With these changes in place, the report reflects the status of each test:

[mjumper@dev-mjumper guacamole-server]$ make check
...
make[4]: Entering directory '/home/mjumper/apache/guacamole/guacamole-server/src/libguac/tests'
PASS: test_libguac 1 - [pool] next_free: OK
PASS: test_libguac 2 - [client] buffer_pool: OK
PASS: test_libguac 3 - [client] layer_pool: OK
PASS: test_libguac 4 - [parser] append: OK
PASS: test_libguac 5 - [parser] read: OK
PASS: test_libguac 6 - [socket] fd_send_instruction: OK
FAIL: test_libguac 7 - [socket] nested_send_instruction: Assertion failed on socket/nested_send_instruction.c:105: CU_ASSERT_EQUAL(offset,strlen(expected))
PASS: test_libguac 8 - [unicode] utf8_charsize: OK
PASS: test_libguac 9 - [unicode] utf8_read: OK
PASS: test_libguac 10 - [unicode] utf8_strlen: OK
PASS: test_libguac 11 - [unicode] utf8_write: OK
PASS: test_libguac 12 - [protocol] decode_base64: OK
============================================================================
Testsuite summary for guacamole-server 1.0.0
============================================================================
# TOTAL: 12
# PASS:  11
# SKIP:  0
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0
============================================================================
See src/libguac/tests/test-suite.log
============================================================================
make[4]: *** [Makefile:1032: test-suite.log] Error 1
make[4]: Leaving directory '/home/mjumper/apache/guacamole/guacamole-server/src/libguac/tests'
make[3]: *** [Makefile:1140: check-TESTS] Error 2
make[3]: Leaving directory '/home/mjumper/apache/guacamole/guacamole-server/src/libguac/tests'
make[2]: *** [Makefile:1211: check-am] Error 2
make[2]: Leaving directory '/home/mjumper/apache/guacamole/guacamole-server/src/libguac/tests'
make[1]: *** [Makefile:879: check-recursive] Error 1
make[1]: Leaving directory '/home/mjumper/apache/guacamole/guacamole-server/src/libguac'
make: *** [Makefile:527: check-recursive] Error 1
[mjumper@dev-mjumper guacamole-server]$ 

The boilerplate I refer to are files like tests/test_libguac.c and tests/protocol/suite.c which do nothing but register test functions with CUnit and request that CUnit run all tests. These changes remove the need to manually define such boilerplate, reducing the burden of writing guacamole-server unit tests to the tests themselves.

[necouchman]

Any idea if a specific version of CUnit is required? I've done an initial review, but when I try to run "make check" on CentOS7, I get the following:

[root@guachost1 guacamole-server-apache]# make check
Making check in src/libguac
make[1]: Entering directory `/root/guacamole-server-apache/src/libguac'
Making check in .
make[2]: Entering directory `/root/guacamole-server-apache/src/libguac'
make[2]: Nothing to be done for `check-am'.
make[2]: Leaving directory `/root/guacamole-server-apache/src/libguac'
Making check in tests
make[2]: Entering directory `/root/guacamole-server-apache/src/libguac/tests'
make  test_libguac
make[3]: Entering directory `/root/guacamole-server-apache/src/libguac/tests'
  CC       client/test_libguac-buffer_pool.o
  CC       client/test_libguac-layer_pool.o
  CC       parser/test_libguac-append.o
  CC       parser/test_libguac-read.o
  CC       pool/test_libguac-next_free.o
  CC       protocol/test_libguac-base64_decode.o
  CC       socket/test_libguac-fd_send_instruction.o
  CC       socket/test_libguac-nested_send_instruction.o
  CC       unicode/test_libguac-charsize.o
  CC       unicode/test_libguac-read.o
  CC       unicode/test_libguac-strlen.o
  CC       unicode/test_libguac-write.o
  GEN      _generated_runner.c
  CC       test_libguac-_generated_runner.o
  CCLD     test_libguac
test_libguac-_generated_runner.o: In function `main':
/root/guacamole-server-apache/src/libguac/tests/_generated_runner.c:88: undefined reference to 
`CU_initialize_registry'
/root/guacamole-server-apache/src/libguac/tests/_generated_runner.c:92: undefined reference to 
`CU_add_suite'
/root/guacamole-server-apache/src/libguac/tests/_generated_runner.c:94: undefined reference to 
`CU_add_test'
/root/guacamole-server-apache/src/libguac/tests/_generated_runner.c:98: undefined reference to 
`CU_add_suite'
/root/guacamole-server-apache/src/libguac/tests/_generated_runner.c:100: undefined reference to 
`CU_add_test'
/root/guacamole-server-apache/src/libguac/tests/_generated_runner.c:101: undefined reference to `CU_add_test'
/root/guacamole-server-apache/src/libguac/tests/_generated_runner.c:105: undefined reference to `CU_add_suite'
/root/guacamole-server-apache/src/libguac/tests/_generated_runner.c:107: undefined reference to 
`CU_add_test'
/root/guacamole-server-apache/src/libguac/tests/_generated_runner.c:108: undefined reference to `CU_add_test'
/root/guacamole-server-apache/src/libguac/tests/_generated_runner.c:109: undefined reference to `CU_add_test'
/root/guacamole-server-apache/src/libguac/tests/_generated_runner.c:110: undefined reference to `CU_add_test'
/root/guacamole-server-apache/src/libguac/tests/_generated_runner.c:114: undefined reference to `CU_add_suite'
/root/guacamole-server-apache/src/libguac/tests/_generated_runner.c:116: undefined reference to `CU_add_test'
/root/guacamole-server-apache/src/libguac/tests/_generated_runner.c:120: undefined reference to `CU_add_suite'
/root/guacamole-server-apache/src/libguac/tests/_generated_runner.c:122: undefined reference to `CU_add_test'
/root/guacamole-server-apache/src/libguac/tests/_generated_runner.c:123: undefined reference to `CU_add_test'
/root/guacamole-server-apache/src/libguac/tests/_generated_runner.c:127: undefined reference to `CU_add_suite'
/root/guacamole-server-apache/src/libguac/tests/_generated_runner.c:129: undefined reference to `CU_add_test'
/root/guacamole-server-apache/src/libguac/tests/_generated_runner.c:130: undefined reference to `CU_add_test'
/root/guacamole-server-apache/src/libguac/tests/_generated_runner.c:142: undefined reference to `CU_set_test_complete_handler'
/root/guacamole-server-apache/src/libguac/tests/_generated_runner.c:143: undefined reference to `CU_run_all_tests'
/root/guacamole-server-apache/src/libguac/tests/_generated_runner.c:147: undefined reference to `CU_cleanup_registry'
/root/guacamole-server-apache/src/libguac/tests/_generated_runner.c:148: undefined reference to `CU_get_error'
client/test_libguac-buffer_pool.o: In function `test_client__buffer_pool':
/root/guacamole-server-apache/src/libguac/tests/client/buffer_pool.c:42: undefined reference to `CU_assertImplementation'
/root/guacamole-server-apache/src/libguac/tests/client/buffer_pool.c:48: undefined reference to `CU_assertImplementation'
/root/guacamole-server-apache/src/libguac/tests/client/buffer_pool.c:53: undefined reference to `CU_assertImplementation'
/root/guacamole-server-apache/src/libguac/tests/client/buffer_pool.c:54: undefined reference to `CU_assertImplementation'
/root/guacamole-server-apache/src/libguac/tests/client/buffer_pool.c:55: undefined reference to `CU_assertImplementation'
client/test_libguac-buffer_pool.o:/root/guacamole-server-apache/src/libguac/tests/client/buffer_pool.c:58: more undefined references to `CU_assertImplementation' follow
collect2: error: ld returned 1 exit status
make[3]: *** [test_libguac] Error 1
make[3]: Leaving directory `/root/guacamole-server-apache/src/libguac/tests'
make[2]: *** [check-am] Error 2
make[2]: Leaving directory `/root/guacamole-server-apache/src/libguac/tests'
make[1]: *** [check-recursive] Error 1
make[1]: Leaving directory `/root/guacamole-server-apache/src/libguac'
make: *** [check-recursive] Error 1

[mike-jumper]

I'll recheck. If things are building, though, this looks more like an issue with linking than with having the wrong version. Was the devel package for CUnit present when ./configure was run?

[necouchman]

Maybe it was my build environment. Went back and did a make distclean followed by ./configure, make -j5, and make check and I no longer get the errors above. Now, instead, I get this:

PASS: test_libguac 1 - [protocol] decode_base64: OK
PASS: test_libguac 2 - [client] buffer_pool: OK
PASS: test_libguac 3 - [client] layer_pool: OK
PASS: test_libguac 4 - [unicode] utf8_charsize: OK
PASS: test_libguac 5 - [unicode] utf8_read: OK
PASS: test_libguac 6 - [unicode] utf8_strlen: OK
PASS: test_libguac 7 - [unicode] utf8_write: OK
PASS: test_libguac 8 - [pool] next_free: OK
PASS: test_libguac 9 - [parser] append: OK
PASS: test_libguac 10 - [parser] read: OK
PASS: test_libguac 11 - [socket] fd_send_instruction: OK
FAIL: test_libguac 12 - [socket] nested_send_instruction: Assertion failed on socket/nested_send_instruction.c:105: CU_ASSERT_EQUAL(offset,strlen(expected))

(Full Output: https://pastebin.com/RLSpPWra)

[mike-jumper]

Yep. That's the test that's been failing all this time, but which (incorrectly) has not been failing the build:

https://issues.apache.org/jira/browse/GUACAMOLE-662

[necouchman]

Aha. Okay, will go ahead and merge these changes.

[necouchman]

Oops...I think your changes to the .gitignore file, here, were a little over-zealous:

nick@linux-i0lj:~/guacamole/guacamole-server> git status
On branch master
Your branch is up-to-date with 'origin/master'.
Untracked files:
  (use "git add <file>..." to include in what will be committed)

    config.guess
    config.sub
    depcomp
    install-sh
    ltmain.sh
    missing
    test-driver

nothing added to commit but untracked files present (use "git add" to track)

Unless you intended to not exclude those anymore?

[mike-jumper]

I believe those have all moved to the build-aux/ subdirectory due to the changes to configure.ac. Do you still see this after git clean -xfd . and autoreconf -fi?

[necouchman]

Ah, okay, looks good.