search

mapbox / protozero

Minimalist protocol buffer decoder and encoder in C++
BSD 2-Clause "Simplified" License
292 stars 78 forks source link

Tests fail #96

Closed AMDmi3 closed 5 years ago

AMDmi3 commented 5 years ago

FreeBSD 12.0 amd64, clang 6.0.1, protozero 1.6.5, protobuf 3.6.1.

unit_tests fail:

% ./unit/unit_tests 

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
unit_tests is a Catch v1.12.0 host application.
Run with -? for options

-------------------------------------------------------------------------------
varint
  encode/decode int32
-------------------------------------------------------------------------------
/ssd/portstrees/sdl_gfx/devel/protozero/work/protozero-1.6.5/test/unit/test_varint.cpp:12
...............................................................................

/ssd/portstrees/sdl_gfx/devel/protozero/work/protozero-1.6.5/test/unit/test_varint.cpp:25: FAILED:
  REQUIRE_FALSE( item.next() )
with expansion:
  !true

-------------------------------------------------------------------------------
varint
  encode/decode uint32
-------------------------------------------------------------------------------
/ssd/portstrees/sdl_gfx/devel/protozero/work/protozero-1.6.5/test/unit/test_varint.cpp:28
...............................................................................

/ssd/portstrees/sdl_gfx/devel/protozero/work/protozero-1.6.5/test/unit/test_varint.cpp:41: FAILED:
  REQUIRE_FALSE( item.next() )
with expansion:
  !true

-------------------------------------------------------------------------------
varint
  encode/decode uint64
-------------------------------------------------------------------------------
/ssd/portstrees/sdl_gfx/devel/protozero/work/protozero-1.6.5/test/unit/test_varint.cpp:44
...............................................................................

/ssd/portstrees/sdl_gfx/devel/protozero/work/protozero-1.6.5/test/unit/test_varint.cpp:57: FAILED:
  REQUIRE_FALSE( item.next() )
with expansion:
  item.next()
due to unexpected exception with message:
  invalid tag exception

===============================================================================
test cases:      49 |      48 passed | 1 failed
assertions: 1052438 | 1052435 passed | 3 failed

reader_tests fail:

...
-------------------------------------------------------------------------------
check assert on non-fixed access to fixed64
-------------------------------------------------------------------------------
/ssd/portstrees/sdl_gfx/devel/protozero/work/protozero-1.6.5/test/t/wrong_type_access/reader_test_cases.cpp:18
...............................................................................

/ssd/portstrees/sdl_gfx/devel/protozero/work/protozero-1.6.5/test/t/wrong_type_access/reader_test_cases.cpp:18: FAILED:
due to unexpected exception with message:
  could not open: 'fixed64/data-zero'

-------------------------------------------------------------------------------
check assert on non-string access to string
-------------------------------------------------------------------------------
/ssd/portstrees/sdl_gfx/devel/protozero/work/protozero-1.6.5/test/t/wrong_type_access/reader_test_cases.cpp:31
...............................................................................

/ssd/portstrees/sdl_gfx/devel/protozero/work/protozero-1.6.5/test/t/wrong_type_access/reader_test_cases.cpp:31: FAILED:
due to unexpected exception with message:
  could not open: 'string/data-string'

-------------------------------------------------------------------------------
check assert on non-fixed access to fixed32
-------------------------------------------------------------------------------
/ssd/portstrees/sdl_gfx/devel/protozero/work/protozero-1.6.5/test/t/wrong_type_access/reader_test_cases.cpp:44
...............................................................................

/ssd/portstrees/sdl_gfx/devel/protozero/work/protozero-1.6.5/test/t/wrong_type_access/reader_test_cases.cpp:44: FAILED:
due to unexpected exception with message:
  could not open: 'fixed32/data-zero'

===============================================================================
test cases:  182 |  16 passed | 166 failed
assertions: 1070 | 624 passed | 446 failed

Most fail because they don't specify .pbf file extension. Full log: https://pastebin.com/zxUgJFy5

joto commented 5 years ago

The files that are being opened are fine, the .pbf extension just isn't shown in the error message.

You should not call the binaries ./unit/unit_tests etc. directly, but call ctest to run the tests. This will make sure it is run from the right directory and it should then find the test files. If you need to run the tests directly, set the TESTS_DIR env variable first.

AMDmi3 commented 5 years ago

You should not call the binaries ./unit/unit_tests etc. directly, but call ctest to run the tests

That was just to demonstrate the problem (actually, properly set up tests don't care which directory they are run form; you can pass CMAKE_CURRENT_{SOURCE|BINARY}_DIR-based path via ADD_DEFINITIONS). It's the same when running ctest.

Full log of cmake .; make -j 5; ctest; ctest -V on a fresh master clone:

https://people.freebsd.org/~amdmi3/protozero.log

Also, as I've mentioned, not all failures are related to data files.

joto commented 5 years ago

Please use an out-of-tree build as is common with CMake and documented in the README.

AMDmi3 commented 5 years ago

It is the same with out of tree build. https://people.freebsd.org/~amdmi3/protozero-oot.log

joto commented 5 years ago

Sorry, I am out of ideas then. Must be something different on the BSD system. I can't reproduce this on Linux and I don't have a FreeBSD system. You have to try to figure this out yourself.

AMDmi3 commented 5 years ago

I've dug a bit, and it seems like either this is catch problem or an undefined behavior somewhere, because varint tests are fixed after commenting out this line in an unrelated test:

https://github.com/mapbox/protozero/blob/b7b290b1a7318fd0b470f07bd4a2f51a3a0af6e8/test/unit/test_basic.cpp#L59

joto commented 5 years ago

Can you compile/run with the UB sanitizer? See the travis config for the needed options: https://github.com/mapbox/protozero/blob/master/.travis.yml#L98-L103.

AMDmi3 commented 5 years ago

It doesn't catch anything

joto commented 5 years ago

I found several places in the code where it is possible we had some undefined behaviour. They are fixed in master now. Can you check?

AMDmi3 commented 5 years ago

Nah, same thing. Honestly I've no idea how to debug this. I suspect this could be a catch problem, so it may be worth trying catch2. I've tried updating catch to 1.12.2, but it had no effect. Catch2 requires syntax changes though, and I have no experience with it.

joto commented 5 years ago

I tried Catch2 in a different project and ran into problems with exceptions. That's why I am staying with Catch1. You can try doing the change mentioned there, I think there was a single place in the Catch1 code where this can be changed.

joto commented 5 years ago

This issue is stale and and I don't see any way forward here. Closing.