ci(local-npm): resume integration testing of NPM canary #7883
Conversation
michaelfig
force-pushed
the
mfig-local-npm
branch
8 times, most recently
from
59cae24 to
f8a4525
Compare
| ################## | ||
| # NPM pack tests | ||
| # Check that all the packages can be packed to publish | ||
| npm-pack: | ||
| needs: build | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| - uses: ./.github/actions/restore-node | ||
| with: | ||
| node-version: '18.x' | ||
|
|
||
| - name: npm pack | ||
| run: yarn lerna exec yarn pack | ||
|
|
There was a problem hiding this comment.
This doesn't actually exercise the same process as lerna publish, so I'm disabling it in favour of the integration test.
| ################## | ||
| # NPM pack tests | ||
| # Check that all the packages can be packed to publish | ||
| npm-pack: | ||
| needs: build | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| - uses: ./.github/actions/restore-node | ||
| with: | ||
| node-version: '18.x' | ||
|
|
||
| - name: npm pack | ||
| run: yarn lerna exec yarn pack | ||
|
|
| @@ -11,7 +11,7 @@ | |||
| "build": "yarn build:bundles", | |||
| "build:bundles": "node scripts/build-bundles.js", | |||
| "prepack": "echo \"export {}; \" | cat - tools/types-ambient.js > tools/types.js && tsc --build jsconfig.build.json", | |||
| "postpack": "git clean -f '*.d.ts*'", | |||
| "postpack": "git clean -f '*.d.ts*' tools/types.js", | |||
There was a problem hiding this comment.
this is more correct. was it also part of an observed problem?
There was a problem hiding this comment.
Yes, leaving behind garbage in the repo after yarn pack.
| @@ -11,7 +11,7 @@ import './prepare-test-env.js'; | |||
| import '@endo/ses-ava/exported.js'; | |||
|
|
|||
| import { wrapTest } from '@endo/ses-ava'; | |||
| import rawTest from 'ava'; | |||
| import rawTest from 'ava'; // eslint-disable-line import/no-extraneous-dependencies | |||
There was a problem hiding this comment.
this seems like a regression caused by the package.json changes
| @@ -284,7 +284,7 @@ export function makeInboundQueueMetrics(initialLength) { | |||
| /** | |||
| * @param {object} param0 | |||
| * @param {any} param0.controller | |||
| * @param {import('@opentelemetry/api-metrics').Meter} param0.metricMeter | |||
| * @param {import('@opentelemetry/api').Meter} param0.metricMeter | |||
There was a problem hiding this comment.
this commit is titled "fix(cosmic-swingset): adapt to new @opentelemetry". For the changelog, there is nothing to fix. This is really a fixup to the change that bumped the OpenTelemetry version. Better to squash it into that commit. Even better is isolating the opentelemetry changes (deps and code) from the other commits.
| @@ -41,7 +41,8 @@ | |||
| }, | |||
| "files": [ | |||
| "*.js", | |||
| "NEWS.md" | |||
| "NEWS.md", | |||
| "src" | |||
There was a problem hiding this comment.
could we do a build instead so we're not back to exporting all the modules?
There was a problem hiding this comment.
I don't see how this is any worse than the status quo. This package.json is not viable: it produces a tarball that doesn't have any implementation. That broke the integration test.
What kind of build do you have in mind? If you just care about exports, let's add packageJson.exports, but I don't want to tackle that in this PR.
There was a problem hiding this comment.
I don't see how this is any worse than the status quo.
It's net better, for sure. But it fails at something that was intended and I wonder if we can make it work as intended.
I had in mind a dist/ style build but I think your packageJson.exports suggestion is better. I agree it can wait for another PR.
michaelfig
force-pushed
the
mfig-local-npm
branch
from
26548e6 to
ff85a7a
Compare
ci(local-npm): resume integration testing of NPM canary
ci(local-npm): resume integration testing of NPM canary
ci(local-npm): resume integration testing of NPM canary
refs: #7879
Description
Reenable integration testing with a local Verdaccio instance to verify the NPM publishing process as well as a dapp consuming the published packages instead of linking the SDK monorepo.
There were several changes to the package configurations needed to make this integration test succeed.
Security Considerations
Scaling Considerations
Documentation Considerations
Testing Considerations