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

Allow tests to reference the key in messages to users #1339

Closed
savage-alex opened this issue Sep 14, 2020 · 18 comments · Fixed by #1341
Closed

Allow tests to reference the key in messages to users #1339

savage-alex opened this issue Sep 14, 2020 · 18 comments · Fixed by #1341

Comments

@savage-alex
Copy link

As a rule creator (and a consumer of the report)
I want to receive verbose messages informing me of the location of an issue with its name
So that I can quickly see what went wrong with my design.

(This makes allot more sense when designing on an unresolved definition and exporting a resolved one into spectral.)

Given the following rule:
image
The consumer is told exactly which parameter has the fault (and the line and ruleset)
image
Perfect right?

Unfortunately with truthy evaluations for a property it doesn't seem possible to get the key out.
I have tried to include the"hint" {parameter} or {value} in the rule:

Rule using property to pull through:
image
Results in:
image

Rule changed to return the value, results in:

image

Is it possible we can get the key returned as an option? I think it would be really helpful to consumers

@P0lip
Copy link
Contributor

P0lip commented Sep 14, 2020

Hey!
Key should be already supported.
Did you try using {property}?

@savage-alex
Copy link
Author

@P0lip hey there, see the above extract when using {{property}} i get a response back of [1] example

Can provide you the API definition and rule if it helps?

@P0lip
Copy link
Contributor

P0lip commented Sep 15, 2020

Ah, it was said {parameter} in the initial description, so I assumed you simply misspelled it, but now I see it returns [1].example.
If I understand the issue correctly, you'd like to have just example printed out, right?
If not, what would be the ideal output for you?

@savage-alex
Copy link
Author

It would be great if it were able to provide the value from another property? such as parameter name?
Given the example below
image
It would be super useful to report the name of the parameter at fault.

@P0lip
Copy link
Contributor

P0lip commented Sep 15, 2020

Agreed. That'd indeed provide a much more meaningful feedback.
I'll try to see what we can do about it.

@savage-alex
Copy link
Author

Trying to get to the point that @arno-di-loreto has with the path / operation tests surfacing really nice responses ala https://apihandyman.io/toolbox/spectral/
image

I have the rules for 400, 401 .... 415 when there is a request body.. But would really level up with the path and operation in the message

@arno-di-loreto
Copy link
Contributor

If the question is “how do I get the path in message “, I use the following conf using path variable on each rule:

message: “{{description}} ({{path}})”

@savage-alex
Copy link
Author

Way to save the day and give me loads of JEST tests to change ;-)
image
Thank you @arno-di-loreto ! Solves one on my problems. What do you think about the top request?

@P0lip
Copy link
Contributor

P0lip commented Sep 15, 2020

@savage-alex I put some draft PR over here #1341, which should address your first concern. I'm not quite sure as to its final shape yet, but either way, please let me know whether it meets your requirements.

@savage-alex
Copy link
Author

Hey @P0lip, im afraid its a bit beyond me to review. Happy to have a quick MS teams call if your game to take a look at it in the flesh?

@P0lip
Copy link
Contributor

P0lip commented Sep 16, 2020

Hey, no worries, you don't need to review it.
I'd just like you to take a look whether the output you desire would be possible to generate once my change is merged.
I can compile a Spectral binary if you want to try it out and don't want to clone the whole Spectral repo.
Does that sound good?

@savage-alex
Copy link
Author

Hey, I have the PR running locally. Given the following OAS parameter against an operation:
parameters:
- in: query
name: includeInactive
description: false by default. If true, includes inactive tenants in the response
schema:
type: boolean
required: false

there is no example. My rule currently sits at:

adv-query-parameters-example-exists:
description: All query parameters SHOULD have an example.
severity: error
message: 'All query parameters SHOULD have an example.'
given: "$..parameters[?(@.in == 'query')]"
then:
field: example
function: truthy

To retrieve the property name:includeInactive in the message as that is where the fault is, what would the message need to have?

@P0lip
Copy link
Contributor

P0lip commented Sep 16, 2020

@savage-alex
Hey, this would be

rules:
  adv-query-parameters-example-exists:
    description: All query parameters SHOULD have an example.
    severity: error
    message: 'Query parameter #{{value.name}} SHOULD have an example.'
    given: "$..parameters[?(@.in == 'query')]"
    then:
      field: example
      function: truthy

@savage-alex
Copy link
Author

savage-alex commented Sep 16, 2020

image
Looks perfect to me! (After I had remembered I had to actually build spectral and use the new binary. Such a n00b I am.

Well done and thank you on the fix! Happy to discuss further on if it will make the cut or not.

@savage-alex
Copy link
Author

savage-alex commented Sep 17, 2020

@P0lip
I have done some more testing this morning and I think this introduced a bug:

  • If you create an example property but fail to give it a value, you get an error
    image

API definition code:
image
You can see where the baseline spectral which is in my VScode has noticed there is a value missing and is highlighting that.

Linting the same file with the baseline finds the value missing and reports the normal error from truthy.

@P0lip
Copy link
Contributor

P0lip commented Sep 17, 2020

@savage-alex could you send me the rule definition?

@savage-alex
Copy link
Author

Sure email or have you got slack or teams so can share there?
Can give you the API def too that way. Rather than filling up GitHub in the clear.

@P0lip
Copy link
Contributor

P0lip commented Sep 18, 2020

@savage-alex
We have Stoplight Slack channel, but I'm also available on APIs You Won't Hate.
You can also send an email to jakub(at)stoplight.io.
Seems like email might be the easiest way?

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 a pull request may close this issue.

3 participants