-
Notifications
You must be signed in to change notification settings - Fork 893
Remove hexdump_test.cc from source list #3094
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
Conversation
@shlmregev I am thinking that running the tests in RELEASE builds is not something we do, as there is no point in running tests that can't tell you if they fail/succeed. So it seems all the changes to debug_log.cc are not needed. |
f5694ef
to
3acafc2
Compare
As a unit test, it belongs on MICROLITE_TEST_SRCS, which gets filtered out of MICROLITE_CC_SRCS. Keeping it on the source list results in it being compiled into the tflm archive, where its main() function conflicts with the application's main. BUG=384562154 modified: tensorflow/lite/micro/tools/make/Makefile
3acafc2
to
9e9a440
Compare
That's a good point and I reverted the changes to debug_log.cc, but the 'x86 Release' CI check is currently failing because I added hexdump_test to the list of tests and DebugVsnprintf() isn't defined. Even if I define it to return 0, hexdump_test will fail. |
I believe that test should only run when Compression is enabled in the config (as we can see Makefile x86 with Compression CI is passing). Is that correct @rkuester ? |
@shlmregev I believe I see the issue now. In |
Compression uses this feature, but the feature and its unit test are independent of compression. So no, I would not conditionally compile out the unit test. It should always run. |
@shlmregev The bluepill build of hexdump_test is still excluded? |
As a unit test, it belongs on MICROLITE_TEST_SRCS, which gets filtered out of MICROLITE_CC_SRCS. Keeping it on the source list results in it being compiled into the tflm archive, where its main() function conflicts with the application's main.
Also, since hexdump.cc calls DebugVsnprintf(), we need to define it in debug_log.cc, also when TF_LITE_STRIP_ERROR_STRINGS is #defined, else compilation fails. Move the #ifdef TF_LITE_STRIP_ERROR_STRINGS to be inside the function implementation, rather around the whole function.
BUG=384562154