fix: example-value-or-externalValue too broad JsonPath expressions

[m-mohr]

Related to #883 (partially, see comments).

Checklist

• Tests added / updated
• Docs added / updated

Does this PR introduce a breaking change?

• Yes
• No

Additional context

$..examples is too broad and incorrectly complains about examples that contain a property examples, see #883 (comment)

Please some one double-check whether I found all paths for the examples in the OAS3 spec. Not sure about Swagger2?!

Edit: I'm not sure why the CI fails. Is it because some JSON path expressions return an empty array? If yes, the old JSON path would also fail on OpenAPI specs with no examples, right?

[nulltoken]

@m-mohr Could you please add a test that would only pass with that change ("examples that contain a property examples") for instance?

[m-mohr]

@nulltoken I could post an example here, but I doubt I can add a good test. I'm not really into your test suite and don't really know what to do there. Could someone make a test out of a json example I post? Or would a new scenario file be enough? 🤔

[nulltoken]

Or would a new scenario file be enough?

@m-mohr That would be perfect!

[nulltoken]

@m-mohr The tests are currently failing because the test documents (incorrectly) expose the examples at the root of the document. They were previously passing because of the (too) broad JsonPath expression.

Could you please update them so that they pass? This would require simple changes in the test documents and expectations.

I'm not really into your test suite and don't really know what to do there.

An example below:

diff --git a/src/rulesets/oas/__tests__/example-value-or-externalValue.ts b/src/rulesets/oas/__tests__/example-value-or-externalValue.ts
index 796ccc0..820f547 100644
--- a/src/rulesets/oas/__tests__/example-value-or-externalValue.ts
+++ b/src/rulesets/oas/__tests__/example-value-or-externalValue.ts
@@ -56,21 +56,21 @@ describe('example-value-or-externalValue', () => {
 
   test('multiple examples - return warnings if missing externalValue and value in one', async () => {
     const results = await s.run({
-      examples: { first: { value: 'value1' }, second: { externalValue: 'external-value2' }, third: {} },
+      components: { examples: { first: { value: 'value1' }, second: { externalValue: 'external-value2' }, third: {} } },
     });
     expect(results).toEqual([
       {
         code: 'example-value-or-externalValue',
         message: 'Example should have either a `value` or `externalValue` field.',
-        path: ['examples', 'third'],
+        path: ['components', 'examples', 'third'],
         range: {
           end: {
-            character: 15,
-            line: 8,
+            character: 17,
+            line: 9,
           },
           start: {
-            character: 12,
-            line: 8,
+            character: 14,
+            line: 9,
           },
         },
         severity: DiagnosticSeverity.Warning,

Note: in order to repro those failure locally, running yarn test "example-value" should work.

[m-mohr]

@nulltoken Thanks for the help regarding the tests. I updated the tests as requested and also managed to add new tests, also checking the

• other paths (parameters, headers, content)
• examples on top level
• examples properties in examples

So I think I made all your requested changes.

[nulltoken]

@m-mohr Looks very good to me.

@P0lip How do you feel about this?

[m-mohr]

Great!
Why's the stoplightio.spectral check hanging?

[P0lip]

Why's the stoplightio.spectral check hanging?

Most likely some permissions issue, as you don't belong to Stoplight organization.
@XVincentX any clue? It's Azure Pipelines.

[P0lip]

Just one comment.
Looking great.

[P0lip]

Thank you!

[XVincentX]

Checking this out @P0lip

[XVincentX]

@P0lip For some reason the build was not triggered — some web hook failed. I've run this manually and it passed the tests, we can just merge it.

[XVincentX]

Ah ok — I think the reason is that this is a PR originated from a fork, not from the repo.