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

added feature to generate test cases based on specification #205

Merged
merged 9 commits into from
Jan 28, 2019

Conversation

BalloonWen
Copy link
Contributor

related issue: #133

-added generate Str based on pattern, maxLength, minLength
-fix bugs generate number
-changed openapi handlerTest template to generate handler test case
-test cases will use ResponseValidator instead of HandlerTestBase, thus removed it.
-now deserialize content string before do validation
-fixed a issue when matching a given uri with schema, now remove query parameters from uri
-changed some testing content
@DSchrupert DSchrupert self-requested a review January 23, 2019 19:20
@DSchrupert
Copy link
Member

Would it be possible to remove the dependency on "generex" and implement that type of random string functionality on your own?

@BalloonWen
Copy link
Contributor Author

This library "generex" is specific for generating String that matches a given regular expression.
Even if I write by myself, it will be similar but may not be better as it's tested by many people.
Unless it has some serious bugs, then we need to consider writing by ourselves.

@stevehu
Copy link
Contributor

stevehu commented Jan 24, 2019

The generex has a dependency automaton which is licensed under BSD 2-clause. It requires we put the copyright and license file somewhere when distributed. I am not sure how to do that yet.

@BalloonWen
Copy link
Contributor Author

@stevehu @NicholasAzar
what should I do then -.-
Should we just give up the pattern part? Not quite sure if it's a common requirement which needs to put a lot of effort on.

@stevehu
Copy link
Contributor

stevehu commented Jan 25, 2019

I really appreciate the effort to make the test cases closer to the real scenarios; however, there is no way to make it perfect as it would be another full-blown project. If we can generate the values based on the correct type(integer/string) with some limitations(length, max, min etc.), it would be good enough. For the developers who are working on the test cases, they need to modify it anyway. What do you think?

@DSchrupert
Copy link
Member

@BalloonWen as we previously discussed, it would be worth including this dependency for this feature (supporting pattern validation) in my opinion

@stevehu
Copy link
Contributor

stevehu commented Jan 25, 2019

@NicholasAzar Do you know how to properly handle the license of automaton which BSD 2 Clause. It is not the direct dependency and it is not part of the light-4j delivery. However, the customers who are using light-4j might be on the hook. To handle this scenario, we have to hire a lawyer to figure out how to handle it properly. So far, we only have Apache 2.0 and MIT licenses in our dependencies. If we include any other licensed library, we are putting our customers in a position that they might be sued.

@ddobrin
Copy link
Contributor

ddobrin commented Jan 25, 2019

If we promote the dependency with this change, we need to advise our respective client(s) before they use this version.

I am not sure whether a software such as BlackDuck should flag whether this dependency creates an issue or not

@ddobrin ddobrin self-requested a review January 28, 2019 13:54
@ddobrin
Copy link
Contributor

ddobrin commented Jan 28, 2019

@stevehu, @NicholasAzar
I have reviewed this change offline with the latest update from @BalloonWen .

It fixes the generation problem and all APIs at my client are generating and building generated code without errors.

This PR has an associated PR in light-rest-4jL:
networknt/light-rest-4j#69

They both can be merged at this time.

Copy link
Contributor

@ddobrin ddobrin left a comment

Choose a reason for hiding this comment

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

As per previous comments

@stevehu stevehu merged commit 73ee49f into networknt:develop Jan 28, 2019
@BalloonWen
Copy link
Contributor Author

The code was changed to add test ignore.
As discussed @ddobrin @stevehu these generated tests should validate response based on the schema, but right after running codegen, there isn't any business to send response yet, which will cause tests failure. Thus, we should add ignore annotation to pass the build. Whenever a developer wants to use this feature, the developer can just remove the annotation.

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.

4 participants