misc: do not publish lighthouse-cli/test except smokehouse #12415
Conversation
| @@ -35,8 +35,9 @@ mkdir -p /tmp/lighthouse-local-test | |||
| cd /tmp/lighthouse-local-test | |||
|
|
|||
| npm init -y | |||
| npm install "$LH_PRISTINE_ROOT/lighthouse-$VERSION.tgz" | |||
| cd node_modules/lighthouse/lighthouse-cli/test/ && npm install lodash.clonedeep && cd ../../../../ | |||
There was a problem hiding this comment.
this command was accidentally installing all our devDeps
idk why I had it cd to that particular folder 🤣
|
|
(they use all their own tests, though) |
|
keeping just the code is only ~50 kB ✅
|
| @@ -22,8 +22,9 @@ lighthouse-logger/ | |||
| !lighthouse-core/scripts/manual-chrome-launcher.js | |||
| !lighthouse-core/scripts/download-chrome.sh | |||
|
|
|||
| # keep smokehouse tests, etc | |||
| !lighthouse-cli/test | |||
There was a problem hiding this comment.
was this a noop previously? it seems like we never actually said to ignore this folder, so why did we negate it... 🤔
There was a problem hiding this comment.
I think it was just to make it explicit, more important when release was just a checklist, not a script, so missing tests might not be noticed.
| @@ -19,7 +19,12 @@ const log = require('lighthouse-logger'); | |||
| const {runSmokehouse} = require('../smokehouse.js'); | |||
| const {server, serverForOffline} = require('../../fixtures/static-server.js'); | |||
|
|
|||
| const coreTestDefnsPath = require.resolve('../test-definitions/core-tests.js'); | |||
| let coreTestDefnsPath = ''; | |||
There was a problem hiding this comment.
does path.join(__dirname, '../test-definitions/core-tests.js') work instead? this try/catch feels a bit awkward
There was a problem hiding this comment.
yes but i was opting to not change the string value.
idc either way. trial by emoji. 👍 for try/catch 👎 for not
connorjclark
changed the title
|
@connorjclark any chance you could land this? Looking at the publisher-ads use of smokehouse in #12415 (comment) I had an idea to make it less ugly, but it would bump into this PR. Plus it's always a good time to trim our install size :) |
ref #8084
afterthought ... would this break something in publisher-ads? maybe we should keep the smokerunner files but get rid of the fixtures n stuff?