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.

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

[JAVA][Rest-assured] add more informations about operations #815

Merged
merged 1 commit into from
Aug 15, 2018

Conversation

viclovsky
Copy link
Contributor

@viclovsky viclovsky commented Aug 14, 2018

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: master, 3.3.x, 4.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

  • Add swagger annotations to client
  • Set default baseReqSpec and baseContextConsumer in ApiClient

See also the description bellow: https://github.com/OpenAPITools/openapi-generator/pull/#issuecomment-413194142

A first version of this PR (using a context class) can be found here: #751

@viclovsky
Copy link
Contributor Author

@bbdouglas (2017/07) @JFCote (2017/08) @sreeshas (2017/08) @jfiala (2017/08) @lukoyanov (2017/09) @cbornet (2017/09) @jeff9finger (2018/01)

@jmini
Copy link
Member

jmini commented Aug 15, 2018

Can you explain the use-case behind adding the swagger annotations?

@viclovsky
Copy link
Contributor Author

viclovsky commented Aug 15, 2018

Hello, @jmini

The main idea is to have ability to get constants from API specification inside client. Constants such as tages, notes, summary etc.
Rest-assured is mostly client for testing. Tests often has integration with test report such as Allure for instance. Test report has detailed information about tests run results, curl arguments, command line etc. And this information is usefull for managers, testers and other team members.
Constants are necessary to fill report easier.
The other thing is that with сonstants we are able to do such things as: grouping tests by tags or make some logs with summaries.

Initially I've made it via public static final constants, but it wasn't convenient to be honest: a lot of constants. So the second solution was adding Context class which combines constants (tags etc.). See PR. But to my mind it was complicating of client.
The final version is to use swagger-annotation and aspects for dealing with tags etc in client.

If you have any idea how I can improve or make it easy it will be very helpfull.

@jmini jmini changed the title [JAVA][Rest-assured] Add swagger annotations to client and set default baseReqSpec and bas… [JAVA][Rest-assured] add more informations about operations Aug 15, 2018
@jmini
Copy link
Member

jmini commented Aug 15, 2018

Thank you for the clarification.

It is a little sad to use a swagger-core version that is made for OpenAPI v2, but for the moment there is no template that creates annotations using a swagger-core version compatible with OpenAPI v3.

We can improve this in the future.


I hope you did understood correctly the reasoning behind my question: I think it is important to have some context behind changes made with a PR.

I took the liberty to update the PR description and the title.

@jmini jmini added this to the 3.2.2 milestone Aug 15, 2018
@jmini jmini merged commit 7b8f51a into OpenAPITools:master Aug 15, 2018
@jmini
Copy link
Member

jmini commented Aug 15, 2018

Follow up PR: #822

A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this pull request Feb 27, 2019
…ols#815)

Add swagger annotations to client and set default baseReqSpec and baseContextConsumer in ApiClient
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants