search

pact-foundation / pact-js

JS version of Pact. Pact is a contract testing framework for HTTP APIs and non-HTTP asynchronous messaging systems.
https://pact.io
Other
1.62k stars 343 forks source link

Upgrade pact js core prebuilds ci #1098

Closed YOU54F closed 1 year ago

YOU54F commented 1 year ago

Includes and tests pact-core 13.15.0 which includes

Requires for completeness

Some notes:-

  1. some binary content issues as per https://github.com/pact-foundation/pact-js-core/issues/447
  2. includes cirrus-ci to test arm64 on linux and macos
  3. updates examples
    • skips mocha-pact and jest-pact examples until they are updated to work with the latest pact-js
    • updates an xml dep as it didn't have binaries for arm64 on the v3/e2e project
    • currently skips plugin tests on windows as verifier is failing, trying to isolate issue in this branch https://github.com/pact-foundation/pact-js/actions/runs/5479635684
    • reinstates macos tests on github actions, resolved one set of tests by increasing mocha timeout (had to update to .mocharc in the process and mocha-opts file is deprecated), removed clean up which was for the old ruby processes.
YOU54F commented 1 year ago

note cirrus-ci failing on PR's

PactBroker::Client::Error - Command `git rev-parse --abbrev-ref HEAD` didn't return anything that could be identified as the current branch.
YOU54F commented 1 year ago

note cirrus-ci failing on PR's

PactBroker::Client::Error - Command `git rev-parse --abbrev-ref HEAD` didn't return anything that could be identified as the current branch.

Resolved by setting GIT_BRANCH and GITCOMMIT via CIRRUS* ENV vars.

info page for cirrus https://cirrus-ci.org/guide/writing-tasks/#environment-variables

best added into the pact-broker client, to capture these for Cirrus-CI users (of which we are for our arm64 builds :) )

YOU54F commented 1 year ago

barring

  1. some doc updates to mention that the node-gyp build system is only required for versions of pact js v10 and v11. v12 will be batteries included,

  2. this yak shave https://github.com/pact-foundation/pact-js/pull/1098/files#r1254869004 which we hadn't identified before as we didn't test on linux arm64

we should be good to go :)

YOU54F commented 1 year ago

Going to get these changes merged in now and released, will get builds green.

We are in a much better place than before, with testing support for additional arches, and finally covering macos in ci again

YOU54F commented 1 year ago

note: as this change also bumps the node engines version, it constitutes a major change,

we could backport this change, without the node engines bump to v11.x, as this will release as v12.x

just a thought, otherwise troubleshooting notes will state v10/v11 of pact-js require the python build chain, v12 onwards doesn't. (it might be wiser leaving it as is, as we have only tested the prebuilds from node 16 onwards, so the engine bump imo makes sense (as is also present in pact-js-core, so a pact-js user on node <16 would probably see an error from pact-js-core)

mefellows commented 1 year ago

Awesome! I agree with getting it in and bumping major version.