feat: add timeout option for both install and test phase #769
Conversation
lib/common-args.js
Outdated
| @@ -73,8 +73,7 @@ module.exports = function commonArgs(app) { | |||
| .option('timeout', { | |||
| alias: 'o', | |||
| type: 'number', | |||
| description: 'Set timeout for npm install', | |||
| default: 1000 * 60 * 10 | |||
| description: 'Set timeout for npm install' | |||
There was a problem hiding this comment.
we use it for test now - maybe a dedicated test timeout makes sense?
Codecov Report
@@ Coverage Diff @@
## master #769 +/- ##
==========================================
- Coverage 96.27% 95.95% -0.33%
==========================================
Files 30 30
Lines 887 889 +2
==========================================
- Hits 854 853 -1
- Misses 33 36 +3
Continue to review full report at Codecov.
|
|
Would love to make some progress here as well - any feedback on it? 🙂 |
|
I've rebased now. Happy to add tests if I could get some guidance on how to best test this |
|
I'm always lost with tests in this repo. I wouldn't be against landing this without new tests. |
|
Feel free to merge 😃 |
|
I cannot merge a draft PR :p |
|
It's not draft anymore 😀 |
|
@targos I was a bit quick there, need to update the code as well. I'll get back to this later tonight |
|
ping @SimenB :) |
Co-authored-by: Michaël Zasso <[email protected]>
|
@targos sorry, completely forgot. Made a search & replace pass now, PTAL 🙂 |
|
I think we shouldn't rename everything to |
SimenB
changed the title
|
@targos what about now? I quite like it as it brings parity between option from |
|
Looks good to me. Thanks, let's ship it! |
|
Yay! |
Checklist
npm testpasseshere
This is #740 without the changes to lookups. I have the same question @Zirro had over there though - where to add tests? Seems like a bunch of the options you can define in
lookup.jsonare untested.