Timeout of test API/cdt/tvthread is now the most common reason for test failures

[krader1961]

Something is wrong with the API/cdt/tvthread unit test since it causes flakey timeout test failures. I'm hoping the problem lies with the design of the test, because if the problem is in the underlying ASO and CDT code used in the test that represents a much more serious issue.

[krader1961]

I ran this test in a loop on my primary macOS server and it failed thusly on the 247'th iteration:

<E> run_test[122]: Stderr for tvthread should have been empty:
        Line=115: FAILED Insert 299 failed [errno=60]

On my OpenSuse 42 VM it timed out on the 330'th iteration. Since these failures are reproducible in a relatively short interval hopefully someone will dig into this issue.

[krader1961]

Not surprising: two recent pushes by myself that trigger Travis CI invocations failed solely because the API/cdt/tvthread unit test timed-out. Why did the previous AST team allow this this state of affairs? Do these timeous represent a real, significant, bug in the CDT or ASO code? Or are they just the random failures one might expect from a unit test that is inherently racey?

[krader1961]

Please note that a unit test should be immune to races unless the unit test is deliberately trying to see if a race condition is a possible problem. That does not seem to be the case for the API/cdt/tvthread unit test failures.

[krader1961]

I'm disabling this unit test. I'm tired of seeing a Travis CI run fail solely because one of the six runs of this test timed-out. We haven't touched the CDT code other than to run it through clang-format. It's unlikely anyone will ever modify it. So I'm not too concerned about skipping this particular test. Still, if anyone cares it would be greatly appreciated if someone were to fix the test.

[krader1961]

I also see I disabled two related tests long ago for similar flakiness. From src/lib/libast/tests/cdt/meson.build:

# TODO: Enable these tests when they are fixed to work reliably. At the moment these
# timeout or fail on most platforms:
#   ['tsafehash.c', 120], ['tsafetree.c', 120],

[krader1961]

In case it isn't clear: Tests that have high rates of false negatives are useless. I'm hoping here that these really are false negatives. That is, the tests are inherently flakey rather than the underlying implementation being tested does not correctly handle concurrency.