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

Binary Content Matching is odd - cross platform/arch #447

Closed
YOU54F opened this issue May 12, 2023 · 9 comments
Closed

Binary Content Matching is odd - cross platform/arch #447

YOU54F opened this issue May 12, 2023 · 9 comments
Labels
bug Indicates an unexpected problem or unintended behavior

Comments

@YOU54F
Copy link
Member

YOU54F commented May 12, 2023

Smells

Cross CI provider and cross arch has shown a few inconsistencies in content-type matching, which is clearly demonstrable here. Haven't thought of a solution just yet, but this at least highlights and allows us to test more combinations than before

Encountered in #444 (comment)

If you have code that looks like this, there is a definite code-smell

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

The condition is used in these tests

it('generates a pact with success', () =>
axios
.request({
baseURL: `http://${HOST}:${port}`,
headers: {
'content-type': usesOctetStream
? 'application/octet-stream'
: 'application/gzip',
Accept: 'application/json',
'x-special-header': 'header',
},

// 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
// Linux: has magic mime matcher, sniffs content
describe('with binary data', () => {
it('generates a pact with success', () => {
const message = pact.newAsynchronousMessage('');
message.expectsToReceive('a binary event');
message.given('some state');
message.givenWithParam('some state 2', 'state2 key', 'state2 val');
message.withBinaryContents(
bytes,
usesOctetStream ? 'application/octet-stream' : 'application/gzip'
);
message.withMetadata('meta-key', 'meta-val');

See pact-foundation/pact-reference#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
Linux: has magic mime matcher, sniffs content

So based on our compat table of

OS Amd64 Arm64
Linux (glibc)
Mac
Windows
Linux (alpine)

✅ = supported
❌ = not supported

We have a few different ways we can test to cover different arch combinations

  • Locally
  • Locally with GitHub Actions / Act
  • GitHub Actions
  • Locally with Cirrus CLI
  • Cirrus CI
OS Amd64 Arm64 Amd64 (GH) Amd64 (GH Act) Amd64 (Cirrus) Arm64 (Cirrus CI) Amd64 (Cirrus CLI) Arm64 (Cirrus CLI)
Linux (glibc)
Mac
Windows
Linux (alpine)

❓ = needs test evidence
❌ = not supported, or untestable on platform

@YOU54F YOU54F added the bug Indicates an unexpected problem or unintended behavior label May 12, 2023
@YOU54F YOU54F changed the title Binary Content Matching is _odd_ cross platform Binary Content Matching is odd - cross platform/arch May 12, 2023
@damusix
Copy link

damusix commented Feb 23, 2024

@YOU54F Running into a similar issue on Mac Arm64:

error @pact-foundation/pact-node@8.6.2: The CPU architecture "arm64" is incompatible with this module.
error Found incompatible module.

@YOU54F
Copy link
Member Author

YOU54F commented Feb 23, 2024

hi this isnt the same issue, you are running an old version of pact-node which has never supported arm64

@YOU54F
Copy link
Member Author

YOU54F commented Feb 23, 2024

you can run pact-node with rosetta or vote on this issue #489 to bring native arm support to pact-node

or migrate to pact-js-core which supports arm64 on macos and linux

@YOU54F
Copy link
Member Author

YOU54F commented Feb 23, 2024

fyi this related issue is where rosetta was supported for pact-node on macos

#264

youll need a version bump to 10.x as described in the linked issue to get it working with rosetta
👍🏽

@damusix
Copy link

damusix commented Mar 1, 2024

@YOU54F Thank you sir, let me look into this

YOU54F added a commit to pact-foundation/pact-reference that referenced this issue Apr 29, 2024
content-type differs for amd64/arm64 macos machines

relates to pact-foundation/pact-js-core#447
@YOU54F
Copy link
Member Author

YOU54F commented May 10, 2024

So it appears installing shared-mime-info in arm64 containers will result in the same behaviour on Linux

Debian

apt install shared-mime-info

Alpine

apk add shared-mime-info

@YOU54F
Copy link
Member Author

YOU54F commented Jun 12, 2024

Fixed as part of pact-foundation/pact-reference#429

will be sorted in 0.4.21 release of the pact_ffi which can then be consumed in here

@YOU54F
Copy link
Member Author

YOU54F commented Jun 25, 2024

Fixed in

@YOU54F YOU54F closed this as completed Jun 25, 2024
@mefellows
Copy link
Member

Amazing work @YOU54F! 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

3 participants