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

tests: Works on travis, fails on my buildbox. #232

Open
leamas opened this issue Sep 24, 2020 · 3 comments · May be fixed by #231
Open

tests: Works on travis, fails on my buildbox. #232

leamas opened this issue Sep 24, 2020 · 3 comments · May be fixed by #231

Comments

@leamas
Copy link

leamas commented Sep 24, 2020

This is really about the #231 PR. However, since the current master doesnt build, there is no way to reproduce test errors there.

On #231, commit 6dd5782, the run_tests completes successfully on travis. However, on my local test box the same test fails, with lots of messages like

timer_tests.short_intervals
/home/mk/cxx-serial/cxx-serial/tests/unit/unix_timer_tests.cc:24: Failure
The difference between r+1 and 0 is 11, which exceeds 1, where
r+1 evaluates to -11,
0 evaluates to 0, and
1 evaluates to 1.

My local box is pretty fast, a Ryzen 7 gen3. OTOH, the Debian machine I use for testing is a vm under VirtualBox.

But then again, I guess the travis build also runs on a vm.

@leamas
Copy link
Author

leamas commented Sep 25, 2020

After porting the tests to cppunit I can run the tests on my native, Fedora without any hypervisor layer. Same results, slightly better formatted:

Test Results:
Run:  2   Failures: 2   Errors: 0


1) test: SerialTimerTest::testOverlappingLongIntervals (F) line: 76 unix_timer_tests.cc
double equality assertion failed
- Expected: -14
- Actual  : -36
- Delta   : 5
- check 2 on 0


2) test: SerialTimerTest::testShortIntervals (F) line: 42 unix_timer_tests.cc
double equality assertion failed
- Expected: 0
- Actual  : -68
- Delta   : 1

It's a bit unfortunate that the delays are random and thus not possible to reproduce.

@leamas
Copy link
Author

leamas commented Sep 25, 2020

The tests are broken since they assume that for example usleep(5000) actually creates a 5 ms delay. However, there are no such guarantees, the delay is never shorter but typically somewhat longer.

The tests needs to be patched taking the actual, elapsed time into account rather than blindly trusting usleep()

EDIT: typos

leamas added a commit to leamas/serial that referenced this issue Sep 25, 2020
Use the cpppunit test framework instead og catkin. This means that the
dependency on both catkin and boost can be dropped to run the tests.

The timer tests are patched to fix the problem described in wjwwood#232.

The new tests needs c++11 chrono features.
@leamas
Copy link
Author

leamas commented Sep 26, 2020

Attaching updated tests, These are cppunit tests, not directly usable in current, catkin context. They also relies on C++11, which somehow makes the timer tests obsolete (since C++11 has platform-indepented timers built in). Nevertheless, enclosed tests works on both Linux and Macos, basically validating the pre-C++11 code with C++11 primitives.

unix_timer_tests.cc.gz

leamas added a commit to leamas/serial that referenced this issue Sep 27, 2020
Compute actually elapsed time instead of assuming that usleep() sleeps
for exactly the right time, which isn't guaranteed.
leamas added a commit to leamas/serial that referenced this issue Sep 27, 2020
Compute actually elapsed time instead of assuming that usleep() sleeps
for exactly the right time, which isn't guaranteed.
leamas added a commit to leamas/serial that referenced this issue Sep 27, 2020
Compute actually elapsed time instead of assuming that usleep() sleeps
for exactly the right time, which isn't guaranteed.
leamas added a commit to leamas/serial that referenced this issue Sep 29, 2020
Compute actually elapsed time instead of assuming that usleep() sleeps
for exactly the right time, which isn't guaranteed.
leamas added a commit to leamas/serial that referenced this issue Sep 29, 2020
Compute actually elapsed time instead of assuming that usleep() sleeps
for exactly the right time, which isn't guaranteed.
leamas added a commit to leamas/serial that referenced this issue Sep 29, 2020
Compute actually elapsed time instead of assuming that usleep() sleeps
for exactly the right time, which isn't guaranteed.
@leamas leamas linked a pull request Sep 29, 2020 that will close this issue
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

Successfully merging a pull request may close this issue.

1 participant