-
Notifications
You must be signed in to change notification settings - Fork 14
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
Fix field name to match Dredd #37
Conversation
Fixes a bug introduced in 0.3.0
@IngmarStein can you provide more context on what the issue is? In the dredd docs it says that the key should be |
Sorry, I could have been clearer but it seems you already found that dredd's documentation was out of sync and that the code actually uses |
To add some detail: goodman 0.3.0 introduced all kinds of validation errors in our tests (mainly optional fields not being optional) and they were fixed by the change in this PR. |
Yea basically I was trying to understand how bad this change was for users since the integration tests still passed. I get this is the right change but I'd like to add additional testing that would would have failed the PR that broke things. Do you have any of the failure output that can help determine what the appropriate tests would be? |
I got many errors similar to
I assume that the The expected response is simply defined as
|
Maybe you can merge this PR and add a regression test in a separate one? |
Yea I thought I would have time sooner rather than later but I'll have to add a test next chance I get. |
Fixes a bug introduced in 0.3.0