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

libcheck timeout testcases and CK_DEFAULT_TIMEOUT/CK_TIMEOUT_MULTIPLIER #181

Open
ladar opened this issue Feb 26, 2019 · 4 comments
Open

Comments

@ladar
Copy link

ladar commented Feb 26, 2019

I'm building and testing libcheck in parallel with a number of other libraries. As a result make check runs when the system is under heavy load. Occasionally see non-timeout test cases fail from timeout problems. I was hoping to solve this issue by setting CK_DEFAULT_TIMEOUT=0 and/or by setting CK_TIMEOUT_MULTIPLIER=10. Unfortunately when I set the troublesome tests pass, but the timeout test cases fail. It appears, (guessing here), that the timeout test cases use hard coded time parameters. Is that correct? Or is there a way to increase the general timeout without causing the timeout test cases to fail?

To avoid the spurious failures, I had to take the drastic step of configuring --disable-timeout-tests. I'm hoping there is a better way, or lacking that, open a bug report, so this issue can be addressed in the future.

@brarcher
Copy link
Contributor

brarcher commented Feb 28, 2019 via email

@ladar
Copy link
Author

ladar commented Feb 28, 2019

@brarcher sadly, I did disable the timeout checks lavabit/magma@a5929a4 in my build script. Disabling the timeout checks, and then setting CK_DEFAULT_TIMEOUT=0 seems to have reliably eliminated failures.

What I was suggesting is that the timeout checks be tweaked to ignore the value of CK_DEFAULT_TIMEOUT/CK_TIMEOUT_MULTIPLIER and/or multiply their respective timeout values by CK_TIMEOUT_MULTIPLIER when that particular environment variable is set.

It just doesn't seem right that those test cases fail if either variable is set, since those variables may be needed to prevent non-timeout tests from failing. That seems to be the case when they're run on a heavily loaded system.

In my case, I succumbed, and disabled timeouts via my build script to avoid the spurious, and random failures. Running a full build, with the dependency checks (which includes libcheck), takes 20 to 60 minutes, so have it fail, checking the log, and then re-running the process is rather painful.

If you don't want to make any of the changes I proposed, then might I suggest detecting when those environment variables have been modified, and disabling/skipping the timeout tests automatically. That seems preferable to having those test cases fail, as failing suggests a problem which doesn't really exist.

As for disabling the timeout tests, it probably won't be an issue for me short term. I've already tested my primary platform many times... but I worry that permanently disabling those tests could be dangerous long term. Specifically, I worry about when either myself, or a user tries using/porting magma to a new platform, and running the tests for the first time. In that situation, the timeout tests might detect a problem, which could be difficulty to identify if encountered randomly.

@ladar
Copy link
Author

ladar commented Feb 28, 2019

P.S. If your curious, you can try running:

git clone https://github.com/lavabit/magma && cd magma && dev/scripts/builders/build.lib.sh all

# And finally, if you want to build/run the magma checks (which depend on libcheck)... 
make check

The Makefile calls build.lib.sh automatically, but skips over the bundled dependencies checks, which is why the script is run explicitly above.

@brarcher
Copy link
Contributor

I agree that the unit tests should ignore the environment variables or react in a more reasonable way. They were designed to not consider the load of the host machine for the timing tests. I'll probably not get around to it, but if someone where interested in making the tests more robust I'd help by reviewing the changes.

Either way, thanks for bringing this up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants