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

Issue 283 #286

Merged
merged 8 commits into from
Nov 7, 2022
Merged

Issue 283 #286

merged 8 commits into from
Nov 7, 2022

Conversation

danielfcollier
Copy link
Contributor

@danielfcollier danielfcollier commented Oct 27, 2022

Issue: #283

The problem has been solved by working only with absolute paths on Windows.

Currently, there are 252 passing tests on Windows and 254 passing tests on Linux (probably MacOS too).

Some tests at:

  • test/specs/invalid/invalid.spec.js
  • test/specs/missing-pointers/missing-pointers.spec.js
  • test/specs/object-source/object-source.spec.js

have been fixed to work on any OS or skipped due to conflicts on Windows (error messages that still keeps the \\ from Windows.)

Three tests at test/specs/http.spec.js have been skipped and it might require opening another issue to investigate further.

@danielfcollier
Copy link
Contributor Author

I will see why the CI has broken.

@danielfcollier
Copy link
Contributor Author

danielfcollier commented Oct 28, 2022

@philsturgeon, please, run the CI again - I'm still trying to understand if the changes are enough to pass the tests pipeline.

I'm trying to run the CI locally to check first here if it is going to pass. But, it is my first try with GitHub Actions (I've just run GitLab Runner), so this is another science...

@philsturgeon
Copy link
Member

philsturgeon commented Oct 31, 2022

Nearly there @danielfcollier, sorry this is a bit labourious. I'll add you as a collaborator so it will let the checks run qithout manual approval.

@philsturgeon philsturgeon disabled auto-merge November 7, 2022 11:35
@philsturgeon philsturgeon merged commit 41ffc9a into APIDevTools:main Nov 7, 2022
@danielfcollier
Copy link
Contributor Author

Hey @philsturgeon, thanks! I will see, looking forward to solve more issues.

jcmosc added a commit to criteria-labs/json-schema-ref-parser that referenced this pull request Jan 4, 2023
philsturgeon pushed a commit that referenced this pull request Jan 11, 2023
* Update package-lock.json file

* Fix broken tests in blank.spec.js

Blank binary file was not getting serialized as { type: "Buffer", data: [] }
because read(file) was returning a string instead of Buffer.

* Fix webpack error when running browser tests

Webpack 4 cannot parse module containing optional chaining operator.
See https://stackoverflow.com/questions/59972341/how-to-make-webpack-accept-optional-chaining-without-babel

* Fix tests in invalid.spec.js when running in Chrome

* Change statusCode to status

* Set the --openssl-legacy-provider flag when running browser tests

Recent versions of node upgraded to OpenSSL 3.0, which deprecated some
older crypto hashing algorithms including md4.

Webpack 4 hard codes the use of md4 ins some places, so we have to set this
flag until we can upgrade Webpack.

* Use isomorphic-fetch instead of node-fetch

Browser tests were failing as polyfill.js was loading a node package in a browser context.

* Increase minimum version of node to 17

This was when the --openssl-legacy-provider option was introduced.
Prior versions of node do not recognize this option.

* Add chokidar@3 as an explicit dev dependency

Browser tests are failing in the CI environment with:
chokidar@3: Error: Cannot find module 'chokidar'

Don't know why but this thread might be relevant:
facebook/create-react-app#10811

* Skip test assertion due to conflict on Windows

See also #286

* Explicitly define browsers and plugins in karma.conf.js

* Replace karma-edge-launcher with newer package
philsturgeon pushed a commit that referenced this pull request Jan 20, 2023
* Update package-lock.json file

* Fix broken tests in blank.spec.js

Blank binary file was not getting serialized as { type: "Buffer", data: [] }
because read(file) was returning a string instead of Buffer.

* Fix webpack error when running browser tests

Webpack 4 cannot parse module containing optional chaining operator.
See https://stackoverflow.com/questions/59972341/how-to-make-webpack-accept-optional-chaining-without-babel

* Fix tests in invalid.spec.js when running in Chrome

* Change statusCode to status

* Set the --openssl-legacy-provider flag when running browser tests

Recent versions of node upgraded to OpenSSL 3.0, which deprecated some
older crypto hashing algorithms including md4.

Webpack 4 hard codes the use of md4 ins some places, so we have to set this
flag until we can upgrade Webpack.

* Use isomorphic-fetch instead of node-fetch

Browser tests were failing as polyfill.js was loading a node package in a browser context.

* Increase minimum version of node to 17

This was when the --openssl-legacy-provider option was introduced.
Prior versions of node do not recognize this option.

* Add chokidar@3 as an explicit dev dependency

Browser tests are failing in the CI environment with:
chokidar@3: Error: Cannot find module 'chokidar'

Don't know why but this thread might be relevant:
facebook/create-react-app#10811

* Skip test assertion due to conflict on Windows

See also #286

* Explicitly define browsers and plugins in karma.conf.js

* Replace karma-edge-launcher with newer package

* Increase default timeout of async tests

* Change package type to module

* Change require/module.exports to import/export in source files

* Delete "use strict" directives from source code

* Update tests to ES modules

* Re-export $RefParser methods as standalone functions

Preserve backwards compatibility

* Make karma config file a .cjs file

Fixes “require() of ES Module …/config.js not supported” error

* Avoid top-level await in path.js

The current configuration of karma uses Webpack 4, which can't parse
modules with top-level await.

* Revert back to __dirname for browser tests

Karma configuration needs to upgrade to Webpack 5 in order to use import.meta.url

* Fix lint issues

* Disable code coverage in CI for now

* Revert "Disable code coverage in CI for now"

This reverts commit f571f0a.

* Replace nyc with c8 to work with ES modules

* Implement a dual CommonJS/ES module package.json

* Hack to allow $RefParser to be imported from require() without .default property

Without this hack, you would have to do this:

const $RefParser = require("@apidevtools/json-schema-ref-parser).default;

Now you can do this:

const $RefParser = require("@apidevtools/json-schema-ref-parser);
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.

2 participants