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

chore(node): run tests against multiple package managers #648

Merged
merged 2 commits into from Oct 26, 2023

Conversation

boneskull
Copy link
Contributor

This changes how lavamoat runs its tests.

To prepare the tests, test/prepare.js is run. This invokes corepack to switch between package managers and performs a clean install and setup using each. The package manager can be changed via environment variable.

Motivation: After changing from Yarn to npm, we were concerned about breaking compat. This should ease our nerves a bit!

Notes:

  • a new policy file was created in project 2 for atob; I'm not sure what caused this change.
  • corepack is seemingly broken in Node.js v14 (segfaults); if this is the case, we just try to use the system package manager

chore(core,node): add some types for scenarios and options

@boneskull
Copy link
Contributor Author

Closes #625 (possibly)

@boneskull boneskull self-assigned this Aug 11, 2023
@boneskull boneskull added the chore overhead, tests, dev env, etc. label Aug 11, 2023
@boneskull
Copy link
Contributor Author

@legobeat I suppose we could also add yarn berry, but I know it won't be so easy. I would like to better understand how exactly this package is consumed by yarn berry users.

@boneskull
Copy link
Contributor Author

boneskull commented Aug 15, 2023

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@boneskull boneskull changed the base branch from boneskull/normalize-test-filenames to boneskull/policy-schema August 15, 2023 20:45
@boneskull boneskull force-pushed the boneskull/smoke branch 2 times, most recently from b4c7ca5 to 9dddc50 Compare September 19, 2023 00:09
@legobeat legobeat dismissed their stale review September 19, 2023 09:57

failing tests resolved

@boneskull boneskull force-pushed the boneskull/smoke branch 3 times, most recently from 8766e95 to e780f61 Compare September 25, 2023 20:39
@naugtur naugtur dismissed their stale review September 26, 2023 07:35

no changes afterall

@naugtur
Copy link
Member

naugtur commented Sep 26, 2023

now that we're no longer supporting node 14 this should be smooth

@boneskull boneskull changed the base branch from main to boneskull/issue616-drop-v14 September 26, 2023 20:15
@boneskull boneskull mentioned this pull request Sep 26, 2023
@boneskull
Copy link
Contributor Author

This now targets #729 and I removed the terrible garbage I had to add for v14.

Base automatically changed from boneskull/issue616-drop-v14 to main October 5, 2023 18:00
@boneskull boneskull force-pushed the boneskull/smoke branch 2 times, most recently from a0c494b to 1643a69 Compare October 5, 2023 21:22
Copy link
Member

@naugtur naugtur left a comment

Choose a reason for hiding this comment

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

I think we forgot about his one. Without node14 it looks nice and clean.

This changes how `lavamoat` runs its tests.

To prepare the tests, `test/prepare.js` is run.  This invokes [corepack](https://npm.im/corepack) to switch between package managers and performs a clean install and setup using each.  The package manager can be changed via environment variable.

Motivation:  After changing from Yarn to npm, we were concerned about breaking compat.  This should ease our nerves a bit!

Notes:

- a new policy file was created in project `2` for `atob`; I'm not sure what caused this change.
- Yarn _must_ have lockfiles or it will get confused about workspaces.
Comment on lines +11 to +15
"test": "npm run test:npm && npm run test:yarn1",
"test:prepare": "node ./test/prepare.js",
"test:npm": "cross-env LAVAMOAT_PM=npm@latest npm run test:prepare && ava",
"test:yarn1": "cross-env LAVAMOAT_PM=yarn@1 npm run test:prepare && ava",
"lint:deps": "depcheck"
Copy link
Contributor

@legobeat legobeat Oct 24, 2023

Choose a reason for hiding this comment

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

Suggested change
"test": "npm run test:npm && npm run test:yarn1",
"test:prepare": "node ./test/prepare.js",
"test:npm": "cross-env LAVAMOAT_PM=npm@latest npm run test:prepare && ava",
"test:yarn1": "cross-env LAVAMOAT_PM=yarn@1 npm run test:prepare && ava",
"lint:deps": "depcheck"
"lint:deps": "depcheck",
"test": "npm run test:npm && npm run test:yarn1",
"test:npm": "cross-env LAVAMOAT_PM=npm@latest npm run test:prepare && ava",
"test:prepare": "node ./test/prepare.js",
"test:yarn1": "cross-env LAVAMOAT_PM=yarn@1 npm run test:prepare && ava"

nit: alphabetic order

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there's a prettier plugin to sort package.json. I suggest we use it

},
"dependencies": {
"keccak": "3.0.0"
},
"devDependencies": {
"patch-package": "^6.2.2"
"patch-package": "6.2.2"
Copy link
Contributor

Choose a reason for hiding this comment

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

As we're pinning, could we do so on 6.5.1? (Latest for ^6.2.2)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to go with "no" for now, because it doesn't work on my machine:

Command failed: /Users/boneskull/projects/lavamoat/lavamoat/packages/node/test/../src/cli.js ./index.js
    Error: No native build was found for platform=darwin arch=arm64 runtime=node abi=115 uv=1 armv=8 libc=glibc node=20.9.0
        loaded from: /Users/boneskull/projects/lavamoat/lavamoat/packages/node/test/projects/2/node_modules/keccak
    
      at load.resolve.load.path (/Users/boneskull/projects/lavamoat/lavamoat/packages/node/test/projects/2/node_modules/node-gyp-build/node-gyp-build.js:61:9)
      at Object.eval (/Users/boneskull/projects/lavamoat/lavamoat/packages/node/test/projects/2/node_modules/keccak/bindings.js:2:73)
      at internalRequire (LavaMoat/core/kernel:896:27)
      at requireRelative (LavaMoat/core/kernel:941:27)
      at requireRelativeWithContext (LavaMoat/core/kernel:909:18)
      at Object.eval (/Users/boneskull/projects/lavamoat/lavamoat/packages/node/test/projects/2/node_modules/keccak/index.js:2:18)
      at internalRequire (LavaMoat/core/kernel:896:27)
      at requireRelative (LavaMoat/core/kernel:941:27)
      at requireRelativeWithContext (LavaMoat/core/kernel:909:18)
      at Object.eval (/Users/boneskull/projects/lavamoat/lavamoat/packages/node/test/projects/2/index.js:11:26)

@@ -30,8 +31,12 @@
"type-fest": "^2.19.0",
"typescript": "^5.2.2"
},
"optionalDependencies": {
"corepack": "0.20.0"
Copy link
Contributor

@legobeat legobeat Oct 24, 2023

Choose a reason for hiding this comment

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

(could) slightly loosen range to not have to manually apply any future non-breaking changes/fixes:

Suggested change
"corepack": "0.20.0"
"corepack": "~0.20.0"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's nothing to upgrade to. corepack is on v0.22.0 now, and requires Node.js v18.

@boneskull boneskull merged commit 7e8e354 into main Oct 26, 2023
11 checks passed
@boneskull boneskull deleted the boneskull/smoke branch October 26, 2023 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore overhead, tests, dev env, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants