Skip to content

Conversation

lacoonte
Copy link
Contributor

@lacoonte lacoonte commented Aug 31, 2025

Fixes #2293 and #2300

@jmartisk
Copy link
Member

jmartisk commented Sep 1, 2025

The code looks good IMO (except you need to run the formatter plugin, the CI is complaining).

About testing... Do you have an idea how to easily test this? I think we don't have suitable infrastructure in place for stopping/starting servers, but AFAIU the retry also applies to when the server returns an error, so we could test it with a server that returns an error on the first invocation and succeeds on the second? Maybe you have a better idea...?

@lacoonte
Copy link
Contributor Author

lacoonte commented Sep 4, 2025

@jmartisk server returning an error is enough for this :) do you have an example of a test class related to this pr? Today i will have some time to finish it

@jmartisk
Copy link
Member

jmartisk commented Sep 4, 2025

@lacoonte, If you're looking for tests that use a similar architecture and can be used for inspiration, look for example at https://github.com/smallrye/smallrye-graphql/blob/main/server/integration-tests/src/test/java/io/smallrye/graphql/tests/client/typesafe/RecordAsInputToTypesafeClientTest.java

Thanks, much appreciated! :)

…safeGraphQLClientProxy

Fixes smallrye#2293
Fixes smallrye#2300

fix checkstyle and added test cases
@lacoonte lacoonte marked this pull request as ready for review September 7, 2025 18:26
@lacoonte
Copy link
Contributor Author

lacoonte commented Sep 7, 2025

@jmartisk Took me more time than i thought as i had a busy week at work. Added two test cases to cover both issues. They fail on the main branch but pass on this one, let me know if there’s anything else I can improve :)

Copy link
Member

@jmartisk jmartisk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot!!!

@jmartisk jmartisk merged commit 46784c8 into smallrye:main Sep 8, 2025
5 checks passed
@github-actions github-actions bot added this to the 2.12.3 milestone Sep 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Retries don’t re-run HTTP call in VertxTypesafeGraphQLClientProxy (eager CompletionStage)

2 participants