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

Trying to release #191

Merged
merged 4 commits into from
May 10, 2022
Merged

Trying to release #191

merged 4 commits into from
May 10, 2022

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants