THRIFT-5877: Add cpp cross tests #3167
Conversation
|
Looks like something was broken in this PR: #3165 |
CJCombrink
changed the title
|
@Jens-G Can this PR please get workflow approval for this PR @fishy Would you please be so kind to give this PR a once over. it is almost a duplicate of the PR that you have approved before but I made a new cleaned up PR trying to be on topic |
Done, didn't see this earlier |
fishy
left a comment
fishy
left a comment
There was a problem hiding this comment.
nice, looks like all crosstests between go and cpp are passing.
|
If I read correctly the PR#3179 should get the cpp-rs tests to also pass. |
|
@Jens-G Is there anything holding this PR back, perhaps some refactoring or moving things around that should be done to get this merged? |
|
@CJCombrink can you please squash your commits? I can merge this after that. |
CJCombrink
force-pushed
the
add_cpp_cross_test
branch
from
2dd5c9d to
ebaf76b
Compare
- Remove usage of v0.16 thrift files for C++ since UUID support was added
- Need to install the locals for some of the unit tests
- Fix UUID support for THeaderProtocol
- Without this the protocol went into an infinite loop due to virtual function calls that recursed to itself
- Best case was a crash, worst case was process got stuck
- Fix UUID support for TProtocolTap
- Sorted the known failures
- Mark cpp and java ssl tests as known failures
CJCombrink
force-pushed
the
add_cpp_cross_test
branch
from
ebaf76b to
8be1faa
Compare
3 participants
Add cross tests to the GitHub pipeline builds
During the work two other issues were identified and implemented/fixed: Missing UUID support for THeaderProtocol and TProtocolTap. This PR includes the fixes for both.
Updated the cross tests to include cpp.
Also updated the known_issues list by adding all java<->cpp ssl related tests
The tests also identified an issue due to #3165 and should be fixed as well
[skip ci]anywhere in the commit message to free up build resources.