Fix 2FA prompt when the package exists #505
Conversation
Done |
|
@sindresorhus Good. And I'd also like the other two approved pull requests (#495, #497) can be merged first. Note that this PR includes changes of #497 now. |
|
@chinesedfan Note that I merged #497, and reviewed #495. |
chinesedfan
changed the title
sindresorhus
changed the title
| test('should not enable 2fa if the package exists', async t => { | ||
| const enable2faStub = sinon.stub(); | ||
| const np = proxyquire('../source', { | ||
| del: sinon.stub(), |
There was a problem hiding this comment.
Why do we need all of these stubs? Can't we run np with --yolo and remove the need for most of them? While I do want the tests to reflect actual stability, I think it's more important in this case to be as specific as possible, and only test what we need to verify - that we skip enabling 2FA when not needed to do so.
There was a problem hiding this comment.
--yolo can only remove del and execa. I used to want to use --preview, but it would skip the enable2fa task totally. And I don't think adding stubs will affect the correctness of the test case. Maybe because I consider it as unit test, while you treat it as end-to-end test.
Fixes regressions in #490.
publishis false.options.existsnow.Can we merge #399 first? It will be easier to write tests.