Skip to content

Conversation

@robertgzr
Copy link
Member

Ref: #31

Signed-off-by: Robert Günzler [email protected]

@dsanders11
Copy link
Contributor

This needs some larger testing refactoring to work - the test binaries are built during build-<os>, so the test binaries from linux won't work on test-alpine.

@robertgzrrobertgzrforce-pushed the test-alpine branch 9 times, most recently from da134cc to 05b5ff4CompareSeptember 29, 2022 16:50
Copy link
Member

@RaisinTenRaisinTen left a comment

Choose a reason for hiding this comment

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

Shouldn't this purely be a circleci config edit?

the test binaries are built during build-<os>, so the test binaries from linux won't work on test-alpine

Why not? When I copy the the dist directory generated in build-linux on my macOS, it does work when I run npm test.

@robertgzr
Copy link
MemberAuthor

@RaisinTen, we're running the test binary:
https://github.com/postmanlabs/postject/blob/main/test/cli.mjs#L66

how does that work with a linux binary on macos?

@RaisinTen
Copy link
Member

Ah I see your point, I was referring to the WASM binary that was getting tested, not the ./build/test/cpp_test binary.

Looks like there are some failures that needs addressing?

@robertgzrrobertgzrforce-pushed the test-alpine branch 2 times, most recently from 378cf54 to 8d30425CompareOctober 12, 2022 09:26
awaitfs.copy("../src/cli.js","../dist/cli.js");
awaitfs.copy("../postject-api.h","../dist/postject-api.h");

cd("..");
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

didn't expect the directory change to persist after exiting the function scope :/ a defer instruction would be useful here

Copy link
Member

Choose a reason for hiding this comment

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

yea, seems to be a quirk of zx that would feel a little unexpected to folks who are used to writing these using sh

Sign up for freeto 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.

4 participants

@robertgzr@dsanders11@RaisinTen