Try to add cpp cross tests #3164

CJCombrink · 2025-06-06T06:51:22Z

This PR tries to enable cpp cross tests

I am not sure if all of this is needed but I need help getting past the ssl issues, I have tried many, many times (Build#25 already) and SSL is outside of my comfort zone.

See some of my comments on my internal PR: CJCombrink#3
It appears to be something about either expired or self signed certificates or both 🤷

Any help will be appreciated

- v0.16 is for those that does not support UUID type but that was added

- It appears that IPv6 is resolved in some cases causing the server to fail to start

- One can consider limiting this

CJCombrink · 2025-06-06T06:52:57Z

.github/workflows/build.yml

      - uses: actions/setup-java@v4
        with:
          distribution: temurin
-          # here we intentionally use an older version so that we also verify Java 17 compiles to it


Is this comment actually true?
I know Java compiled on an old version should work with newer versions: This is not the first time I run into the issue with a old version trying something compiled with a new version.
The logs print this:

Exception in thread "main" java.lang.UnsupportedClassVersionError: org/apache/thrift/test/TestClientKt has been compiled by a more recent version of the Java Runtime (class file version 61.0), this version of the Java Runtime only recognizes class file versions up to 52.0

Which proves the statement wrong

the comment about java 17 is probably a typo? the one we want to make sure is probably java 7 not 17?

I can't remember the exact details but I think we do want to maintain compatibility with older lts java versions.

cc @jimexist to chime in.

it was not a typo.

that UnsupportedClassVersionError likely from kotlin lib which isn't configured with the same compiler flags as the java lib project?

@jimexist Thanks for dropping in and the update: I want to drop this change from my PR since it does not belong, but what is the correct fix for this that should go into the repo to ensure that java/kotlin tests pass without this issue?

I created a ticket: THRIFT-5879: java and kotlin cross tests fail in the GitHub action

CJCombrink · 2025-06-06T06:55:42Z

lib/cpp/test/OneWayHTTPTest.cpp


  {
-    std::shared_ptr<TSocket> socket(new TSocket("localhost", port));
+    std::shared_ptr<TSocket> socket(new TSocket("127.0.0.1", port));


This appears to be an issue with localhost getting resolved to the IPv6 address ::1 and stuff breaks.
I am actually reconsidering this commit and instead for the pipeline purge the IPv6 ::1 localhost from the /etc/hosts file in the pipeline.

Or one that knows more about IPv6 and the socket libs used can perhaps see if this can be fixed in the library; I don't know enough of IPv6 and the libraries used to know if this is even an option

test/cpp/src/TestClient.cpp

.github/workflows/build.yml

fishy · 2025-06-06T15:35:29Z

.github/workflows/build.yml

      - uses: actions/setup-java@v4
        with:
          distribution: temurin
-          # here we intentionally use an older version so that we also verify Java 17 compiles to it


the comment about java 17 is probably a typo? the one we want to make sure is probably java 7 not 17?

I can't remember the exact details but I think we do want to maintain compatibility with older lts java versions.

fishy

lgtm modulo the java version change

CJCombrink · 2025-06-10T05:07:37Z

lgtm modulo the java version change

@fishy Thanks for you review thus far.
As mentioned, I want to drop the java change but something needs to be fixed to fix the java/kotlin cross tests.

The C++ SSL cross tests all still fail and I need some help to get past that. The worst case would be to mark them as known failures but ideally I would like to solve the issue.

CJCombrink · 2025-06-11T07:00:46Z

Specifically the part that I expect to work that is not is the java Server and C++ Client for SSL.

For example in the cross tests logs (re-ordered them a bit to match pairs)

...
java-cpp                binary            zlib-ip                  success
java-cpp                binary            zlib-ip-ssl              failure(-6)
...
java-cpp                binary            framed-ip                success
java-cpp                binary            framed-ip-ssl            failure(-6)
...

fishy · 2025-06-11T15:36:12Z

There's probably some known issue with java cross test ssl handling. If you look at other cross tests, or history or cross tests of prior runs, it seems to only work between java-java and java-kotlin, and never worked with other languages.

(by "never worked" I actually meant "have not worked for a looooong time")

CJCombrink · 2025-06-12T05:19:29Z

There's probably some known issue with java cross test ssl handling

Thanks for the info, it does in deed look like a known issue.
I am trying to spend some time to figure out why does cpp <-> cpp not all work and then perhaps see where I am at and maybe mark the ssl ones as know issues.

- 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

- Looked at all the protocols to check if they had the correct calls

CJCombrink · 2025-06-12T11:14:35Z

lib/cpp/src/thrift/protocol/THeaderProtocol.cpp

  return proto_->writeBinary(str);
 }

+uint32_t THeaderProtocol::writeUUID(const TUuid& uuid) {


@Jens-G This is a bugfix for the THeaderProtocol.
Can I just create a Jira Ticket for it and we close it part of this PR (once I get there) or do I need to pull this out into a new PR for the issue?

Can I just create a Jira Ticket for it and we close it part of this PR

I think either way is fine.

CJCombrink · 2025-06-12T11:16:06Z

lib/cpp/src/thrift/protocol/TProtocolTap.h

    return rv;
  }

+  uint32_t readUUID(TUuid& uuid) {


I just saw this as missing, I will see if it makes sense to add a test for this protocol since I see there aren't any

- Easier to follow what was added and what not

CJCombrink · 2025-06-12T18:57:27Z

I am starting to clean this up and split out the relevant parts to relevant PRs.

@jimexist I still need some guidance on what is the correct fix for the kotlin error (#3164 (comment))

@Jens-G Can I bundle the two fixes into one PR that also adds the cross tests (#3164 (comment), #3164 (comment))

CJCombrink · 2025-06-12T19:23:51Z

Hoping for some answers but closing this PR for the more cleaned up #3167

CJCombrink and others added 9 commits June 5, 2025 08:38

Use latest thrift file for cpp

5329796

- v0.16 is for those that does not support UUID type but that was added

Remove lisp from building by default

5ae0e1a

Add cpp cross tests

63a37e7

Be explicit about localhost ip

c29b4ec

- It appears that IPv6 is resolved in some cases causing the server to fail to start

Missing locales for the cpp tests

9521348

Try to upload everything that is needed for the cpp tests

f673f4d

Missing libraries for cpp tests

f4cf10e

- One can consider limiting this

Update java version for cross test

54a2210

Trying to debug the cpp ssl issues

764e213

CJCombrink commented Jun 6, 2025

View reviewed changes

test/cpp/src/TestClient.cpp Show resolved Hide resolved

CJCombrink commented Jun 6, 2025

View reviewed changes

test/cpp/src/TestClient.cpp Show resolved Hide resolved

fishy reviewed Jun 6, 2025

View reviewed changes

fishy added c++ github_actions Pull requests that update GitHub Actions code labels Jun 6, 2025

fishy requested a review from jimexist June 6, 2025 16:00

fishy approved these changes Jun 6, 2025

View reviewed changes

CJCombrink added 2 commits June 12, 2025 11:10

UUID support for THeaderProtocol

f8e6ec8

- 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

UUID support for TProtocolTap

a4aee68

- Looked at all the protocols to check if they had the correct calls

CJCombrink commented Jun 12, 2025

View reviewed changes

CJCombrink added 2 commits June 12, 2025 12:18

Just sort the known failures

dbb2c52

- Easier to follow what was added and what not

Mark cpp and java ssl tests as known failures

5b067a7

CJCombrink closed this Jun 12, 2025

CJCombrink mentioned this pull request Jun 19, 2025

THRIFT-5877: Add cpp cross tests #3167

Merged

5 tasks

CJCombrink deleted the test/add_cpp_cross_tests branch November 6, 2025 06:01

Try to add cpp cross tests #3164

Try to add cpp cross tests #3164

Uh oh!

Conversation

CJCombrink commented Jun 6, 2025

Uh oh!

CJCombrink Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fishy Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

fishy Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

jimexist Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CJCombrink Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

CJCombrink Jun 12, 2025

Choose a reason for hiding this comment

Uh oh!

CJCombrink Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fishy Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

fishy left a comment

Choose a reason for hiding this comment

Uh oh!

CJCombrink commented Jun 10, 2025

Uh oh!

CJCombrink commented Jun 11, 2025

Uh oh!

fishy commented Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CJCombrink commented Jun 12, 2025

Uh oh!

CJCombrink Jun 12, 2025

Choose a reason for hiding this comment

Uh oh!

Jens-G Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CJCombrink Jun 12, 2025

Choose a reason for hiding this comment

Uh oh!

CJCombrink commented Jun 12, 2025

Uh oh!

CJCombrink commented Jun 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

CJCombrink Jun 6, 2025 •

edited

Loading

jimexist Jun 6, 2025 •

edited

Loading

CJCombrink Jun 6, 2025 •

edited

Loading

fishy commented Jun 11, 2025 •

edited

Loading

Jens-G Jun 12, 2025 •

edited

Loading