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 Response Validator to validate response content #69

Merged
merged 4 commits into from Jan 28, 2019

Conversation

BalloonWen
Copy link
Contributor

@BalloonWen BalloonWen commented Jan 23, 2019

related issue: networknt/light-codegen#133
resolved issue: #27

-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
@stevehu
Copy link
Contributor

stevehu commented Jan 24, 2019

@BalloonWen @ddobrin I have linked this PR to the open issue #27 as the PR is resolving it. Thanks.

@ddobrin
Copy link
Contributor

ddobrin commented Jan 24, 2019

@stevehu: @BalloonWen and I have discussed this and there are multiple aspects which have to be added for the request validator.

The code requires also a unit test for it.

This code will be tested first, as discussed with @BalloonWen and then additional comments will be provided in due 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.

This PR has been tested offline together with its associated change in the light-codegen.
It can be merged at this time.

It is missing a test case and issue:
#27
... is not addressed completely. That issue requires also a handler, test cases as well as the ability to add validation in the client.

Nevertheless, the gap here can be handled as part of resolving issue #27 and this PR should be merged together with the light-codegen one

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.

None yet

3 participants