-
Notifications
You must be signed in to change notification settings - Fork 66
Conversation
src/smoke-tests/smoke-tests.js
Outdated
}; | ||
|
||
before(() => { | ||
if (process.env.DEPLOY_KEY) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DEPLOY_KEY
is already configured on Travis and it associates to a test account I created on Zapier.com.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool! is that anywhere in 1P or somewhere others will be able to access it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! You can find it in 1P. The account is [email protected]. The deploy key can be found there in the Form Details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what an awesome PR!! A couple of little organizational things. Feel free to fix and/or ignore if you disagree.
package.json
Outdated
@@ -28,6 +28,7 @@ | |||
"lint": "node_modules/.bin/eslint zapier.js src", | |||
"lint-snippets": "node_modules/.bin/eslint snippets --rule 'no-unused-vars: 0' --ignore-pattern snippets/next.js", | |||
"test": "npm run build && NODE_ENV=test node_modules/mocha/bin/mocha -t 30000 --recursive lib/tests && npm run lint && npm run lint-snippets", | |||
"smoke-test": "npm run build && NODE_ENV=test node_modules/mocha/bin/mocha -t 30000 --recursive lib/smoke-tests", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
node_modules/mocha/bin/mocha
can just be mocha
(same with above line)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, might make a pretest
script that runs build and run it manually from here (it'll get picked up before test
automatically)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice suggestion! Fixed in 00f2354.
src/smoke-tests/smoke-tests.js
Outdated
}; | ||
|
||
before(() => { | ||
if (process.env.DEPLOY_KEY) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool! is that anywhere in 1P or somewhere others will be able to access it?
src/smoke-tests/smoke-tests.js
Outdated
} | ||
} | ||
context.package.version = context.package.filename.match( | ||
/\d+\.\d+\.\d+/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be worth moving this regex into a constant so it can be enforced the same way across the package (since I bet we use the same pattern elsewhere)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made it a constant in 00f2354. I couldn't find anywhere else in the cli repo that uses this regex. So I think we can keep it in this file (smoke-tests.js
) for now.
}); | ||
const baselineSize = res.headers.get('content-length'); | ||
const newSize = fs.statSync(context.package.path).size; | ||
newSize.should.be.within(baselineSize * 0.7, baselineSize * 1.3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very cool 😍
"Smoke tests" - these are some sanity tests I've been doing manually every time I release a new version of the npm package. This is to make sure there're no mistakes during packaging, which can't be captured in unit tests. Let's automate these smoke tests so we can be more confident about releasing.
Going to do the same to core and schema as well.