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

Add auto-generated test files #2276

Open
7 of 20 tasks
wing328 opened this issue Feb 29, 2016 · 12 comments
Open
7 of 20 tasks

Add auto-generated test files #2276

wing328 opened this issue Feb 29, 2016 · 12 comments

Comments

@wing328
Copy link
Contributor

wing328 commented Feb 29, 2016

As part of #1921, we've add capabilities to generate test files for API and model files.

If anyone has cycle to add the feature to the following languages, please reply to let us know:

Keywords: unit tests, integration tests

@ePaul
Copy link
Contributor

ePaul commented Feb 29, 2016

I don't quite understand: What is the purpose of those generated tests?
The PHP tests seem to be empty, so they are not yet testing anything – It looks as after generation they need to be edited to actually test something. But then the "don't edit this file" comment should go away.

But then what is being tested here? The getters/setters of the model objects? (That kind of test, if deemed necessary, should also be generated. Though I would (as a codegen-user, not as a maintainer of codegen) prefer if the generated code just works and I don't have to test it myself.)

What might be useful would be a test of the actual API methods (which is to be run from a test framework against an actual server) – generating some template as a start for your own written tests.

@wing328
Copy link
Contributor Author

wing328 commented Mar 1, 2016

The purpose is to save developer's time in creating those test files from scratch. (I'll remove "don't edit this file" in my next PR)

And yes, ideally we should test getter/setter for each model. The work I've done is just a starting point. Ideally the test fils should leverage all available information provided in the spec to generate all possible test cases.

In my opinion, generating the test code makes the SDK/API client looks more mature. For example, if there are 2 open-source Ruby SDKs for Twitter API - one with test cases and another without any, I would tend to think the one with test cases is more mature. What do you think?

There are scaffolded test cases for API methods, e.g. https://github.com/swagger-api/swagger-codegen/blob/master/samples/client/petstore/csharp/SwaggerClientTest/Lib/SwaggerClient.Test/PetApiTests.cs. Is that what you're looking for?

Thanks again for the feedback. I think what I'll do is to update the wording to elaborate more on the purpose of the scaffolded test cases in the auto-generated test files.

@wing328
Copy link
Contributor Author

wing328 commented Mar 5, 2016

@ePaul does it make sense to you?

Later if anyone from the community has cycle, we can update codegen to automatically generate test cases specified by vendor extension in the endpoint definition.

@wing328
Copy link
Contributor Author

wing328 commented Mar 5, 2016

@ePaul FYI. #2301 to revise the wording has been merged into master.

@wing328 wing328 removed the Client: R label Mar 6, 2016
@ePaul
Copy link
Contributor

ePaul commented Mar 11, 2016

@wing328 Thanks, this way it is clearer.

@waiterZen
Copy link

Hi, I want to contribute javascript related test code. And I also want to add some feature after driven down the core if I find something can make improvement. If I choose JS, all the pr will commit to this reps?

@wing328
Copy link
Contributor Author

wing328 commented Apr 13, 2016

@waiterZen that's a separate project. You can refer to the PRs above to get a better understanding of what the change (adding auto-generated test files) looks like

@mhuisman
Copy link

in java/retrofit2 there is sufficient functionality in ApiClient.mustache to merit adding a default template that offers unit tests for that functionality. I'm shoehorning it into into api_test.mustache but it feels imprecise/ugly. Would you be interested in a PR addressing adding ApiClient_test.mustache?

@wing328
Copy link
Contributor Author

wing328 commented Mar 15, 2017

Would you be interested in a PR addressing adding ApiClient_test.mustache?

Definitely. Please kindly file a PR so that we can review the enhancement. Thanks.

@wing328
Copy link
Contributor Author

wing328 commented Apr 19, 2017

@mhuisman if you need help with the PR, please reply to let us know.

@cbornet
Copy link
Contributor

cbornet commented Apr 19, 2017

In that case please do it against branch 2.3.0 since there were some changes in the API.

@grokify
Copy link

grokify commented Jun 7, 2018

Given the merged pull requests, what else is necessary for Go to get a ☑?

[ ] Go (#2532, #2577)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment