Skip to content

Conversation

@philsturgeon
Copy link
Member

@philsturgeon philsturgeon commented Dec 6, 2021

Trying to get this releases as there are laods of recent improvements, including OAS 3.1 support, but I am getting a few problems.

I've had to remove npm run update from release because of various dependency hell problems around libsass, npm-check and uuid being triggered by audit fix. We can solve that later.

With that removed I'm seeing the following test fail:

 API with circular (recursive) $refs ✔ should parse successfully ✔ should resolve successfully ✔ should dereference successfully 1) should validate successfully 23 passing (258ms) 1 failing 1) API with circular (recursive) $refs should validate successfully: Error: Error resolving $ref pointer "/Users/phil/src/swagger-parser/test/specs/circular/person.yaml". "/Users/phil/src/swagger-parser/test/specs/circular/person.yaml" not found. at $Refs._resolve (node_modules/@apidevtools/json-schema-ref-parser/lib/refs.js:153:11) at resolveIf$Ref (node_modules/@apidevtools/json-schema-ref-parser/lib/pointer.js:237:41) at Pointer.resolve (node_modules/@apidevtools/json-schema-ref-parser/lib/pointer.js:103:5) at $Ref.resolve (node_modules/@apidevtools/json-schema-ref-parser/lib/ref.js:124:20) at $Refs._resolve (node_modules/@apidevtools/json-schema-ref-parser/lib/refs.js:156:15) at dereference$Ref (node_modules/@apidevtools/json-schema-ref-parser/lib/dereference.js:133:23) at crawl (node_modules/@apidevtools/json-schema-ref-parser/lib/dereference.js:62:28) at crawl (node_modules/@apidevtools/json-schema-ref-parser/lib/dereference.js:71:30) at crawl (node_modules/@apidevtools/json-schema-ref-parser/lib/dereference.js:71:30) at crawl (node_modules/@apidevtools/json-schema-ref-parser/lib/dereference.js:71:30) at crawl (node_modules/@apidevtools/json-schema-ref-parser/lib/dereference.js:71:30) at crawl (node_modules/@apidevtools/json-schema-ref-parser/lib/dereference.js:71:30) at dereference (node_modules/@apidevtools/json-schema-ref-parser/lib/dereference.js:19:22) at SwaggerParser.validate (lib/index.js:175:11) at async Context.<anonymous> (test/specs/circular/circular.spec.js:42:17) 

I'm a little out of the loop and honestly don't know if this is broken or working as expected and expectations changed.

@JamesMessinger sorry to bug you but could you rescue me from this rabbithole?

@srbarba
Copy link

srbarba commented Dec 8, 2021

@philsturgeon

Maybe I'm wrong, but given error seems to be related to APIDevTools/json-schema-ref-parser#199

The test is validating circular.yaml at test/specs/circular, wich references to definitions/person.yaml, which references to itself as person.yaml.

The problem is that json-schema-ref-parser is looking for person.yaml internal references as test/specs/circular/person.yaml instead of test/specs/circular/definitions/person.yaml.

I assume some upgraded dependency (maybe js-yaml?) is breaking this relative paths.

If you run release script in main branch without upgrading dependencies, the test is passing.

I'm getting other errors with adyen specs, but I think this errors are not related with parser but with specs.

I'm new with swagger-parser library and I'm interested on getting OpenApi 3.1 support.
Let me know if I can help any way.

@robraux
Copy link

robraux commented Jan 11, 2022

The testing issue here was introduced with the update to >= json-schema-ref-parser@9.0.7. If you run this release branch against a forced version 9.0.6 you'll see the tests pass (well they fail on access of api.apis.guru, but that site appears inaccessible ATM).

This PR APIDevTools/json-schema-ref-parser#195 appears to introduce the breaking change, specifically around the dereferencedCache addition.

Using the current json-schema-ref-parser@9.0.9 package and commenting out dereferencedCache.set($refPath, dereferencedObject); allows tests to pass for example, (see: https://github.com/APIDevTools/json-schema-ref-parser/blob/748608401888e475fe052a634ee46d0ed0a54c02/lib/dereference.js#L177)

I don't know enough about the interdependencies of these packages to dig deeper for a course of resolution but am also eager to see OAS 3.1 support for this package.

@philsturgeon
Copy link
Member Author

@P0lip hey Jakub, I know this is outside of jurisdiction but can you help me figure out the changes to make here? @robraux has some suggestions but I don't want to break things with false positives.

@philsturgeon philsturgeon merged commit 553770b into main May 10, 2022
@philsturgeon philsturgeon deleted the release branch May 10, 2022 21:54
@github-pages github-pages bot temporarily deployed to github-pages May 10, 2022 21:55 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

5 participants