test(package): add npm pack install fixtures#112
Conversation
|
Warning Review limit reached
More reviews will be available in 39 minutes and 25 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (18)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds a set of isolated “install-from-npm pack tarball” fixtures plus a runner/CI step to validate the published package shape (entrypoints/exports/peers/optional adapters) without changing runtime behavior—aligned with Issue #67’s goal of catching packaging regressions that unit tests can’t.
Changes:
- Added seven fixture projects under
test/fixtures/*to validate CJS/ESM entrypoints, “no default export”, Backbone shim import, optional jQuery DomApi usage, Vite build, and minimum underscore peer. - Added
test/fixtures/run.mjsandnpm run test:fixturesto build, pack to a temp dir, install the tarball into each fixture, run validation, and clean generated state. - Wired the fixture run into CI after coverage on Node 24.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
test/fixtures/run.mjs |
Orchestrates build → pack → per-fixture install + validate + cleanup. |
test/fixtures/cjs-node/package.json |
CJS fixture manifest + validate script. |
test/fixtures/cjs-node/validate.cjs |
Asserts require('marionette') exposes View and MnObject. |
test/fixtures/esm-node/package.json |
ESM fixture manifest + validate script. |
test/fixtures/esm-node/validate.mjs |
Asserts import * as Mn exposes View/MnObject and no default. |
test/fixtures/no-default-export/package.json |
“No default export” fixture manifest + validate script. |
test/fixtures/no-default-export/validate.mjs |
Asserts default import fails in a controlled subprocess. |
test/fixtures/shim/package.json |
Backbone shim fixture manifest + validate script. |
test/fixtures/shim/validate.mjs |
Verifies marionette/backbone patches Backbone’s triggerMethod. |
test/fixtures/jquery-dom-api/package.json |
jQuery DomApi fixture manifest + local jquery/jsdom deps. |
test/fixtures/jquery-dom-api/validate.mjs |
Verifies jQuery DomApi makes view.$() return a jQuery collection. |
test/fixtures/peer-underscore-min/package.json |
Pins underscore to 1.13.0 for minimum-peer validation. |
test/fixtures/peer-underscore-min/validate.mjs |
Confirms underscore version and that ESM import exposes View. |
test/fixtures/vite/package.json |
Vite build fixture manifest + vite build validator. |
test/fixtures/vite/index.html |
Minimal Vite entry HTML for build validation. |
test/fixtures/vite/src/main.js |
Imports from marionette to ensure Vite can bundle the packed package. |
package.json |
Adds test:fixtures script and includes fixtures in base lint scope. |
.github/workflows/ci.yml |
Runs npm run test:fixtures after coverage in CI. |
| cleanFixture(fixtureDir); | ||
| run('npm', ['install'], { cwd: fixtureDir }); | ||
| run('npm', ['install', '--no-save', tarballPath], { cwd: fixtureDir }); | ||
| run('npm', ['run', 'validate'], { cwd: fixtureDir }); | ||
| cleanFixture(fixtureDir); |
| assert.ok(result instanceof $); | ||
| assert.strictEqual(result[0].textContent, 'child'); | ||
| assert.strictEqual(Object.prototype.hasOwnProperty.call(view, '$el'), false); |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 18 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="package.json">
<violation number="1" location="package.json:65">
P2: `test/fixtures/` will not be linted for the new `.mjs` validator and runner files, so `lint:ci` misses the added fixture code.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| "coverage": "vitest run --coverage", | ||
| "coveralls": "npm run coverage && cat coverage/lcov.info | coveralls", | ||
| "lint:base": "eslint backbone.js config/ index.js jquery-dom-api.js mixins/ modules/ test/setup/vitest.js test/unit/ utils/ vitest.config.js", | ||
| "lint:base": "eslint backbone.js config/ index.js jquery-dom-api.js mixins/ modules/ test/fixtures/ test/setup/vitest.js test/unit/ utils/ vitest.config.js", |
There was a problem hiding this comment.
P2: test/fixtures/ will not be linted for the new .mjs validator and runner files, so lint:ci misses the added fixture code.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At package.json, line 65:
<comment>`test/fixtures/` will not be linted for the new `.mjs` validator and runner files, so `lint:ci` misses the added fixture code.</comment>
<file context>
@@ -62,10 +62,11 @@
"coverage": "vitest run --coverage",
"coveralls": "npm run coverage && cat coverage/lcov.info | coveralls",
- "lint:base": "eslint backbone.js config/ index.js jquery-dom-api.js mixins/ modules/ test/setup/vitest.js test/unit/ utils/ vitest.config.js",
+ "lint:base": "eslint backbone.js config/ index.js jquery-dom-api.js mixins/ modules/ test/fixtures/ test/setup/vitest.js test/unit/ utils/ vitest.config.js",
"lint": "npm run lint:base -- --fix",
"lint:ci": "npm run lint:base -- --max-warnings=0",
</file context>
Closes #67
Summary
npm run test:fixturesto build, pack, install the tarball into each fixture, run each fixture validator, and clean generated fixture state.Public Behavior Boundary
marionette/backboneimport.marionette/jquery-dom-apiimport. The fixture validates the shipped adapter behavior:view.$()returns a jQuery collection while$elremains absent because the adapter does not providewrapEl.Fixture List
test/fixtures/cjs-node:require("marionette")exposesViewandMnObject.test/fixtures/esm-node:import * as Mn from "marionette"exposesViewandMnObject, with no default export.test/fixtures/no-default-export: default import fails in a controlled subprocess.test/fixtures/shim: explicitmarionette/backboneimport patches Backbone.test/fixtures/jquery-dom-api: jQuery DomApi works when jQuery is installed locally.test/fixtures/vite:vite buildsucceeds against the packed package.test/fixtures/peer-underscore-min: ESM import works withunderscore@1.13.0.Package/Export Behavior Verified
.tgzwithnpm install --no-save, not source aliases.Validation Commands and Results
npm run lint:cipassed.npm run buildpassed. Existing Browserslist freshness and Rollup circular dependency warnings were emitted.npm packpassed and listed the expected dist/package files.npm run test:fixturespassed for all seven fixtures. Existing Browserslist/Rollup warnings were emitted during the internal build; npm also emitted a transitivewhatwg-encodingdeprecation warning from the jsdom fixture install.ruby -e "require 'yaml'; YAML.load_file('.github/workflows/ci.yml'); puts 'workflow yaml ok'"passed.CI Integration Notes
npm run test:fixturesafter coverage on Node 24.Scope Creep Check
logs/modified.Follow-up Issues
Summary by cubic
Adds
npm packinstall fixtures and anpm run test:fixturesworkflow to validate the publishedmarionettepackage across key usage scenarios. CI now runs these checks after coverage to catch packaging/export issues.require), ESM (import * as), no default import,marionette/backboneshim,marionette/jquery-dom-apiadapter withjquery,vitebuild, and minimumunderscore@1.13.0.test:fixturesscript to build, pack, install the tarball into each fixture, run validators, and clean state.npm run test:fixtureson Node 24 after coverage.marionette/backbone, jQuery usage only viamarionette/jquery-dom-api, and that core requires onlyunderscore.Written for commit f462a94. Summary will update on new commits.