Fix CI tests - Githubissues

APIDevTools / json-schema-ref-parser

Parse, Resolve, and Dereference JSON Schema $ref pointers in Node and browsers

https://apitools.dev/json-schema-ref-parser

MIT License

952 stars 227 forks source link

Fix CI tests #294

Closed jcmosc closed 1 year ago

jcmosc commented 1 year ago

Fixes CI tests, as much as possible from a forked repo.

Note: This PR bumps the minimum Node version from 12 to 17. This is because browser tests use an older version of Webpack that requires NODE_OPTIONS=--openssl-legacy-provider to be specified when using recent versions of Node. However this option does not exist in older version of Node.

philsturgeon commented 1 year ago

thanks for this! nearly there, do you know whats up with ubuntu browser tests?

jcmosc commented 1 year ago

It's failing because it can't launch Edge.

09 01 2023 11:25:29.297:INFO [launcher]: Starting browser Edge
09 01 2023 11:25:29.298:ERROR [launcher]: No binary for Edge browser on your platform.
  Please, set "EDGE_BIN" env variable.

To be honest I was hoping there would be something different about the Sauce Labs configuration in this repo than my fork that would make this work. Will need to investigate further.

But maybe we don't need it for Edge anyway as it's included in the runner image? https://github.com/actions/runner-images/blob/main/images/linux/Ubuntu2204-Readme.md

philsturgeon commented 1 year ago

Anything we can do to get away from SauceLabs is a good thing IMO. It's been a nightmare trying to keep it happy.

jcmosc commented 1 year ago

Ok that's good to know.

These commits should now pass. https://github.com/criteria-labs/json-schema-ref-parser/actions/runs/3888908655

I got rid of Sauce Labs and browsers are now just tested on their "home" platform, i.e. Edge on windows, Safari on macOS. Though there is no runner for macos-latest because the karma launcher for Safari doesn't work (it hasn't been updated in 9 years). So there are no Safari tests at all.

coveralls commented 1 year ago

Pull Request Test Coverage Report for Build 3889025475

6 of 8 (75.0%) changed or added relevant lines in 3 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 94.632%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
lib/resolvers/http.js	4	6	66.67%
<!--	Total:	6	8	75.0%	-->

Totals
Change from base Build 3554282655:	0%
Covered Lines:	841
Relevant Lines:	876

💛 - Coveralls

zomars commented 1 year ago

This is breaking most apps that rely on Node 16:

Elhebert commented 1 year ago

We also had our app breaking without warning due to this.

I feel like this should have been released as a new major version. Changing the minimum Node version is a breaking change after all.