Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add module for end 2 end tests for verify the the generated client co… #252

Merged
merged 1 commit into from
Dec 12, 2023
Merged

Add module for end 2 end tests for verify the the generated client co… #252

merged 1 commit into from
Dec 12, 2023

Conversation

zettelmj
Copy link
Collaborator

…mplies with the spec and is run able

@zettelmj
Copy link
Collaborator Author

@cjbooms I would propose to add acceptance/end2end tests like this.
As I am working on the jdk client this comes handy to make sure that both versions of the generated client code work the same way. For the new client I would like to run the same set of tests with both clients.

@cjbooms
Copy link
Owner

cjbooms commented Nov 25, 2023

@cjbooms I would propose to add acceptance/end2end tests like this.
As I am working on the jdk client this comes handy to make sure that both versions of the generated client code work the same way. For the new client I would like to run the same set of tests with both clients.

Yep, fine with me. Nice that it's a separate module

@zettelmj zettelmj force-pushed the end2end-test-for-http-clients branch from ec8a20a to c601935 Compare November 26, 2023 17:49
zettelmj
zettelmj commented Nov 26, 2023
View reviewed changes
end2end-tests/src/test/kotlin/com/cjbooms/fabrikt/clients/okio3/Okio3Test.kt Outdated
examplePath2Client.getExamplePath2PathParam("304", 10)
}

assertThat(result.message).isEqualTo("[304]: ")
Copy link
Collaborator Author

@zettelmj zettelmj Nov 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@cjbooms This looks dodgy. There seems to be no handling for the 3xx cases.
Messages are either successful -> 200..299, bad request -> 400..499 or server error -> 500..599.
Everything 3xx falls through into the else case in http-util.kt.hbs line 46.

Copy link
Owner

Choose a reason for hiding this comment

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

I don't think anyone at Zalando have used the API client for anything other than tests, so probably proper 3X handling never came up.

@zettelmj zettelmj force-pushed the end2end-test-for-http-clients branch 3 times, most recently from ba48788 to cb57996 Compare November 29, 2023 21:07
@zettelmj zettelmj force-pushed the end2end-test-for-http-clients branch from cb57996 to 910d361 Compare November 29, 2023 21:48
@zettelmj zettelmj marked this pull request as ready for review November 29, 2023 21:56
@zettelmj
Copy link
Collaborator Author

First part of the coverage done

@zettelmj zettelmj requested a review from cjbooms November 29, 2023 21:57
@zettelmj
Copy link
Collaborator Author

#257

@cjbooms
Copy link
Owner

cjbooms commented Dec 12, 2023

👍

@cjbooms cjbooms merged commit e6300c9 into cjbooms:master Dec 12, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjbooms cjbooms cjbooms approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zettelmj @cjbooms