Fix Errors on Windows introduced by #358 #375
Conversation
gxuu
force-pushed
the
oct-win-warn
branch
2 times, most recently
from
54d41b3 to
2a5dad6
Compare
| @@ -1 +1,10 @@ | |||
| add_test_executable(test_ymq test_ymq.cpp) | |||
There was a problem hiding this comment.
I think for all platforms, ymq tests should passed.
When add support for windows, it should make changes that will enable windows, then add matrix
So, please revert this
There was a problem hiding this comment.
The thing is when developing locally this creates error because this is Linux specific code (as of now).
We still want the tests that can be build (for example this test_ymq_logging)to build so that we can test along developing.
Windows build for YMQ has always been enabled and it didn't report problem only because it's not part of the CI.
So I think it's more reasonable to turn it on after test_ymq is cross platformed as well, preferably at the same time as CI for Windows is on. The name test_ymq may be confusing in cross platformed context, because now it really is just test_ymq_linux.
Please read above, if you still insist, then I will revert.
Signed-off-by: gxu <[email protected]>
Signed-off-by: gxu <[email protected]>
as OSS doesn't build on Windows Signed-off-by: gxu <[email protected]>
Signed-off-by: gxu <[email protected]>
Signed-off-by: gxu <[email protected]>
Signed-off-by: gxu <[email protected]>
Signed-off-by: gxu <[email protected]>
Signed-off-by: gxu <[email protected]>
Signed-off-by: gxu <[email protected]>
| AllowShortFunctionsOnASingleLine: InlineOnly | ||
| AllowAllParametersOfDeclarationOnNextLine: true | ||
| BinPackParameters: OnePerLine | ||
| # BinPackParameters: OnePerLine |
There was a problem hiding this comment.
This is creating problem on Visual Studio - it doesn't know about this option. Perhaps the bundled clang format version is not new enough. If this needs to be reverted, I will revert it.
gxuu
changed the title
|
|
||
| # Post build: We need to copy if dll is not present, otherwise the test program won't find gtest.dll | ||
| # See https://stackoverflow.com/questions/69978314/cmake-with-gtest-on-windows-build-starts-test-but-shared-libs-cannot-be-found | ||
| if (WIN32) |
There was a problem hiding this comment.
Be noted that gtest has been set to always be linked dynamically. Also I think it's unwise to link statically when it comes to test cases. Not sure whether we still want to link everything statically. But this is an easier change to first make sure the test is runnable. Perhaps we revisit it later.
2 participants
READY FOR REVIEW
See commits title.
closes #376