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

CI: Test across Node 16/18/20 + arm64 MacOS test #444

Merged
merged 13 commits into from
May 12, 2023
58 changes: 58 additions & 0 deletions .cirrus.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
env:
YOU54F marked this conversation as resolved.
Show resolved Hide resolved
PACT_BROKER_FEATURES: publish_pacts_using_old_api

BUILD_TEST_TASK_TEMPLATE: &BUILD_TEST_TASK_TEMPLATE
arch_check_script:
- uname -am
test_script:
- node --version
- script/ci/build-and-test.sh

# These are probably expected to fail in the post install script
# until we are packing v2.0.0 of pact-ruby-standalone that supports
# arm64 linux - as per https://github.com/pact-foundation/pact-js-core/issues/416
# Error: Error while locating pact binary: Cannot find binary for platform 'linux' with architecture 'arm64'.
# linux_arm64_task:
# skip: "changesInclude('.github/**')"
# env:
# matrix:
# - IMAGE: node:16-slim
# - IMAGE: node:18-slim
# - IMAGE: node:20-slim
# arm_container:
# image: $IMAGE
# install_script:
# - apt update --yes && apt install --yes curl python3 make build-essential g++ unzip zip
# << : *BUILD_TEST_TASK_TEMPLATE

linux_amd64_task:
env:
matrix:
- IMAGE: node:16-slim
- IMAGE: node:18-slim
- IMAGE: node:20-slim
container:
image: $IMAGE
install_script:
- apt update --yes && apt install --yes curl python3 make build-essential g++ unzip zip
<< : *BUILD_TEST_TASK_TEMPLATE


mac_task:
macos_instance:
image: ghcr.io/cirruslabs/macos-ventura-base:latest
env:
PACT_BROKER_FEATURES: publish_pacts_using_old_api
NVS_HOME: ${HOME}/.nvs
PATH: ${NVS_HOME}:${PATH}
matrix:
- NODE_VERSION: 16
- NODE_VERSION: 18
- NODE_VERSION: 20
install_script: # we need to install rosetta as v1.x of pact-ruby-standalone doesn't support arm64
- softwareupdate --install-rosetta --agree-to-license
- brew install nvm
- source $(brew --prefix nvm)/nvm.sh
- nvm install $NODE_VERSION
- nvm use $NODE_VERSION
<< : *BUILD_TEST_TASK_TEMPLATE
56 changes: 11 additions & 45 deletions .github/workflows/build-and-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,64 +8,30 @@ on:

jobs:
build-and-test-osx:
runs-on: macos-latest
runs-on: ${{ matrix.os }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AWESOME!! I had been meaning to do this for ages, but never got around to figuring out what the syntax was.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have been making some mental foo of late, you can generate matrices dynamically in your actions, so I have a run of pact ffi/ pact verifier cli and pact mock server cli, for all the targets, in one super clean action.

I just wish you could report on individual matrix combos, because you could build up an awesome compat matrix, with github badges.

The other way is to have separate workflows , I need to check if you can pull in one master workflow and then have loads of mini workflows, that just specify x combo (node 14 / windows) so they appear as seperate workflows, and you could get a status badge for each one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just wish you could report on individual matrix combos, because you could build up an awesome compat matrix, with github badges.

I dunno - I think it's better to just report the whole success / failure - as then you don't get tempted to continue with "well, it doesn't work on everything we deploy to".

It would be nice to be able to generate the list of tested combinations, though!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a whole just fail isn’t helpful for me finding the needle in the haystack and github’s view of multi matrix runs needs some work, detail gets lots and you just have 12 boxes that al look the same from the truncated description until you click on one, and that is an expensive use of time

i need to check out github blocks again as there is meant to be some cool stuff you can do in readmes now

defaults:
run:
shell: bash
strategy:
fail-fast: false
matrix:
node-version: [16.x]
node-version: [16,18,20]
os: [macos-latest,ubuntu-latest,windows-latest]

steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v3
- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v1
uses: actions/setup-node@v3
with:
node-version: ${{ matrix.node-version }}
- run: script/ci/build-and-test.sh
if: runner.os != 'Windows'
env:
NODE_VERSION: ${{ matrix.node-version }}
PACT_BROKER_FEATURES: publish_pacts_using_old_api

build-and-test-ubuntu:
runs-on: ubuntu-latest
strategy:
matrix:
node-version: [16.x]

steps:
- uses: actions/checkout@v2
- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v1
with:
node-version: ${{ matrix.node-version }}
- name: Set MSVS version
run: npm config set msvs_version 2017
- run: script/ci/build-and-test.sh
YOU54F marked this conversation as resolved.
Show resolved Hide resolved
if: runner.os == 'Windows'
env:
YOU54F marked this conversation as resolved.
Show resolved Hide resolved
NODE_VERSION: ${{ matrix.node-version }}
PACT_BROKER_FEATURES: publish_pacts_using_old_api

build-and-test-windows:
runs-on: windows-latest
strategy:
matrix:
node-version: [16.x]

steps:
- uses: actions/checkout@v2
- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v1
with:
node-version: ${{ matrix.node-version }}
- name: Fix node-gyp
run: |-
npm install --global node-gyp@latest
npm prefix -g | % {npm config set node_gyp "$_\node_modules\node-gyp\bin\node-gyp.js"}
shell: pwsh
- run: bash script/ci/build-and-test.sh
shell: bash
env:
NODE_VERSION: ${{ matrix.node-version }}
# The windows build agent has trouble unpacking the tar for
# linux, so we only download windows binaries. This means
# we cannot release from Windows in CI.
ONLY_DOWNLOAD_PACT_FOR_WINDOWS: true
YOU54F marked this conversation as resolved.
Show resolved Hide resolved
PACT_BROKER_FEATURES: publish_pacts_using_old_api
8 changes: 7 additions & 1 deletion DEVELOPER.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
Do this and you should be 👌👌👌:

```
bash script/download-libs.sh
npm ci
npm test
```
```


_notes_ - As a developer, you need to run `bash script/download-libs.sh` to download the FFI libraries prior to running `npm install` / `npm ci` as the libraries will be expected to be there, and you won't have any `node_modules` installed yet.

For end users, the `ffi` folder is populated, as part of the npm publishing step.
3 changes: 2 additions & 1 deletion script/ci/build-and-test.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#!/bin/bash -eu
set -eu # This needs to be here for windows bash, which doesn't read the #! line above
set -e # This needs to be here for windows bash, which doesn't read the #! line above
set -u

SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")"; pwd)" # Figure out where the script is running
. "$SCRIPT_DIR"/../lib/robust-bash.sh
Expand Down
2 changes: 1 addition & 1 deletion script/lib/download-ffi.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ if [[ $(find "${FFI_DIR}" -name "${FFI_VERSION}*") ]]; then
fi

warn "Cleaning ffi directory $FFI_DIR"
rm -rf "${FFI_DIR:?}/*"
YOU54F marked this conversation as resolved.
Show resolved Hide resolved
rm -rf "${FFI_DIR:?}"
mkdir -p "$FFI_DIR/osxaarch64"
mkdir -p "$FFI_DIR/linuxaarch64"

Expand Down
20 changes: 15 additions & 5 deletions test/consumer.integration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ const { expect } = chai;
const HOST = '127.0.0.1';

const isWin = process.platform === 'win32';
const isLinux = process.platform === 'linux';
const isDarwinArm64 = process.platform === 'darwin' && process.arch === 'arm64';
const isLinuxArm64 = process.platform === 'linux' && process.arch === 'arm64';
const isCirrusCi = process.env['CIRRUS_CI'] === 'true';
const usesOctetStream =
isLinuxArm64 || isWin || isDarwinArm64 || (isCirrusCi && isLinux);

describe('FFI integration test for the HTTP Consumer API', () => {
setLogLevel('trace');
Expand Down Expand Up @@ -73,7 +79,9 @@ describe('FFI integration test for the HTTP Consumer API', () => {
.request({
baseURL: `http://${HOST}:${port}`,
headers: {
'content-type': 'application/octet-stream',
'content-type': usesOctetStream
? 'application/octet-stream'
: 'application/gzip',
Accept: 'application/json',
'x-special-header': 'header',
},
Expand Down Expand Up @@ -168,9 +176,11 @@ describe('FFI integration test for the HTTP Consumer API', () => {

// See https://github.com/pact-foundation/pact-reference/issues/171 for why we have an OS switch here
// Windows: does not have magic mime matcher, uses content-type
// OSX on CI: does not magic mime matcher, uses content-type
// OSX: has magic mime matcher, sniffs content
// OSX arm64: does not magic mime matcher, uses content-type
// OSX x86_64: has magic mime matcher, sniffs content
// OSX x86_64 - CI GitHub Actions: has magic mime matcher, sniffs content
// Linux: has magic mime matcher, sniffs content
// Linux - CI Cirrus: does not have magic mime matcher, uses content-type
describe('with binary data', () => {
beforeEach(() => {
pact = makeConsumerPact(
Expand All @@ -188,7 +198,7 @@ describe('FFI integration test for the HTTP Consumer API', () => {
interaction.withQuery('someParam', 0, 'someValue');
interaction.withRequestBinaryBody(
bytes,
isWin ? 'application/octet-stream' : 'application/gzip'
usesOctetStream ? 'application/octet-stream' : 'application/gzip'
);
interaction.withResponseBody(
JSON.stringify({
Expand All @@ -211,7 +221,7 @@ describe('FFI integration test for the HTTP Consumer API', () => {
.request({
baseURL: `http://${HOST}:${port}`,
headers: {
'content-type': isWin
'content-type': usesOctetStream
? 'application/octet-stream'
: 'application/gzip',
Accept: 'application/json',
Expand Down
12 changes: 7 additions & 5 deletions test/message.integration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,12 @@ chai.use(chaiAsPromised);
const { expect } = chai;

const isWin = process.platform === 'win32';
const isOSX = process.platform === 'darwin';
const isCI = process.env['CI'] === 'true';
const isLinux = process.platform === 'linux';
const isDarwinArm64 = process.platform === 'darwin' && process.arch === 'arm64';
const isLinuxArm64 = process.platform === 'linux' && process.arch === 'arm64';
const isCirrusCi = process.env['CIRRUS_CI'] === 'true';
const usesOctetStream =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes me sad. Not your fault of course. We need to find a solution to this!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know, I'll card this up for a yak shave, now have we have a way of testing repeatably across platform, we can sort it, or document it

isLinuxArm64 || isWin || isDarwinArm64 || (isCirrusCi && isLinux);
YOU54F marked this conversation as resolved.
Show resolved Hide resolved

const getFeature = async (address: string, protoFile: string) => {
const def = await load(protoFile);
Expand Down Expand Up @@ -100,9 +104,7 @@ describe('FFI integration test for the Message Consumer API', () => {
message.givenWithParam('some state 2', 'state2 key', 'state2 val');
message.withBinaryContents(
bytes,
isWin || (isOSX && isCI)
? 'application/octet-stream'
: 'application/gzip'
usesOctetStream ? 'application/octet-stream' : 'application/gzip'
);
message.withMetadata('meta-key', 'meta-val');

Expand Down