diff options
author | nlf <quitlahok@gmail.com> | 2021-02-02 22:48:27 +0300 |
---|---|---|
committer | Ruy Adorno <ruyadorno@hotmail.com> | 2021-02-05 17:43:12 +0300 |
commit | d44393929a3a3ce82f18a60e1cc9bb23c970fe19 (patch) | |
tree | 6edc4afbb9ab105d24218ffe7c9903da7e36950d /test | |
parent | c1589c160e95700c3dad9467a045d998bb8c23c8 (diff) |
chore: utils cleanup and tests
- remove spawn util, refactor help command
- add tests for read-user-info, minor refactor
- add tests for pulse-till-done util, refactor
- add tests for otplease util
- add tests for open-url util
- remove unused no-progress-while-running util
PR-URL: https://github.com/npm/cli/pull/2601
Credit: @nlf
Close: #2601
Reviewed-by: @ruyadorno, @wraithgar
Diffstat (limited to 'test')
-rw-r--r-- | test/lib/help.js | 52 | ||||
-rw-r--r-- | test/lib/utils/open-url.js | 165 | ||||
-rw-r--r-- | test/lib/utils/otplease.js | 94 | ||||
-rw-r--r-- | test/lib/utils/pulse-till-done.js | 35 | ||||
-rw-r--r-- | test/lib/utils/read-user-info.js | 116 |
5 files changed, 460 insertions, 2 deletions
diff --git a/test/lib/help.js b/test/lib/help.js index 17018acc6..40a035421 100644 --- a/test/lib/help.js +++ b/test/lib/help.js @@ -55,12 +55,13 @@ const glob = (p, cb) => { let spawnBin = null let spawnArgs = null +let spawnCode = 0 const spawn = (bin, args) => { spawnBin = bin spawnArgs = args const spawnEmitter = new EventEmitter() process.nextTick(() => { - spawnEmitter.emit('close', 0) + spawnEmitter.emit('exit', spawnCode) }) return spawnEmitter } @@ -76,7 +77,9 @@ const help = requireInject('../../lib/help.js', { '../../lib/utils/npm-usage.js': npmUsage, '../../lib/utils/open-url.js': openUrl, '../../lib/utils/output.js': output, - '../../lib/utils/spawn.js': spawn, + child_process: { + spawn, + }, glob, }) @@ -339,6 +342,29 @@ test('npm help ?(un)star', t => { }) }) +test('npm help - woman viewer propagates errors', t => { + npmConfig.viewer = 'woman' + spawnCode = 1 + globResult = [ + '/root/man/man1/npm-star.1', + '/root/man/man1/npm-unstar.1', + ] + t.teardown(() => { + npmConfig.viewer = undefined + spawnCode = 0 + globResult = globDefaults + spawnBin = null + spawnArgs = null + }) + + return help(['?(un)star'], (err) => { + t.match(err, /help process exited with code: 1/, 'received the correct error') + t.equal(spawnBin, 'emacsclient', 'maps woman to emacs correctly') + t.strictSame(spawnArgs, ['-e', `(woman-find-file '/root/man/man1/npm-unstar.1')`], 'passes the correct arguments') + t.end() + }) +}) + test('npm help un*', t => { globResult = [ '/root/man/man1/npm-unstar.1', @@ -360,3 +386,25 @@ test('npm help un*', t => { t.end() }) }) + +test('npm help - man viewer propagates errors', t => { + spawnCode = 1 + globResult = [ + '/root/man/man1/npm-unstar.1', + '/root/man/man1/npm-uninstall.1', + '/root/man/man1/npm-unpublish.1', + ] + t.teardown(() => { + spawnCode = 0 + globResult = globDefaults + spawnBin = null + spawnArgs = null + }) + + return help(['un*'], (err) => { + t.match(err, /help process exited with code: 1/, 'received correct error') + t.equal(spawnBin, 'man', 'calls man by default') + t.strictSame(spawnArgs, ['1', 'npm-unstar'], 'passes the correct arguments') + t.end() + }) +}) diff --git a/test/lib/utils/open-url.js b/test/lib/utils/open-url.js new file mode 100644 index 000000000..ce1783dad --- /dev/null +++ b/test/lib/utils/open-url.js @@ -0,0 +1,165 @@ +const { test } = require('tap') +const requireInject = require('require-inject') + +const npm = { + _config: { + json: false, + browser: true, + }, + config: { + get: (k) => npm._config[k], + set: (k, v) => { + npm._config[k] = v + }, + }, +} + +const OUTPUT = [] +const output = (...args) => OUTPUT.push(args) + +let openerUrl = null +let openerOpts = null +let openerResult = null +const opener = (url, opts, cb) => { + openerUrl = url + openerOpts = opts + return cb(openerResult) +} + +const openUrl = requireInject('../../../lib/utils/open-url.js', { + '../../../lib/npm.js': npm, + '../../../lib/utils/output.js': output, + opener, +}) + +test('opens a url', (t) => { + t.teardown(() => { + openerUrl = null + openerOpts = null + OUTPUT.length = 0 + }) + openUrl('https://www.npmjs.com', 'npm home', (err) => { + if (err) + throw err + + t.equal(openerUrl, 'https://www.npmjs.com', 'opened the given url') + t.same(openerOpts, { command: null }, 'passed command as null (the default)') + t.same(OUTPUT, [], 'printed no output') + t.done() + }) +}) + +test('returns error for non-https and non-file url', (t) => { + t.teardown(() => { + openerUrl = null + openerOpts = null + OUTPUT.length = 0 + }) + openUrl('ftp://www.npmjs.com', 'npm home', (err) => { + t.match(err, /Invalid URL/, 'got the correct error') + t.equal(openerUrl, null, 'did not open') + t.same(openerOpts, null, 'did not open') + t.same(OUTPUT, [], 'printed no output') + t.done() + }) +}) + +test('returns error for non-parseable url', (t) => { + t.teardown(() => { + openerUrl = null + openerOpts = null + OUTPUT.length = 0 + }) + openUrl('git+ssh://user@host:repo.git', 'npm home', (err) => { + t.match(err, /Invalid URL/, 'got the correct error') + t.equal(openerUrl, null, 'did not open') + t.same(openerOpts, null, 'did not open') + t.same(OUTPUT, [], 'printed no output') + t.done() + }) +}) + +test('opens a url with the given browser', (t) => { + npm.config.set('browser', 'chrome') + t.teardown(() => { + openerUrl = null + openerOpts = null + OUTPUT.length = 0 + npm.config.set('browser', true) + }) + openUrl('https://www.npmjs.com', 'npm home', (err) => { + if (err) + throw err + + t.equal(openerUrl, 'https://www.npmjs.com', 'opened the given url') + t.same(openerOpts, { command: 'chrome' }, 'passed the given browser as command') + t.same(OUTPUT, [], 'printed no output') + t.done() + }) +}) + +test('prints where to go when browser is disabled', (t) => { + npm.config.set('browser', false) + t.teardown(() => { + openerUrl = null + openerOpts = null + OUTPUT.length = 0 + npm.config.set('browser', true) + }) + openUrl('https://www.npmjs.com', 'npm home', (err) => { + if (err) + throw err + + t.equal(openerUrl, null, 'did not open') + t.same(openerOpts, null, 'did not open') + t.equal(OUTPUT.length, 1, 'got one logged message') + t.equal(OUTPUT[0].length, 1, 'logged message had one value') + t.matchSnapshot(OUTPUT[0][0], 'printed expected message') + t.done() + }) +}) + +test('prints where to go when browser is disabled and json is enabled', (t) => { + npm.config.set('browser', false) + npm.config.set('json', true) + t.teardown(() => { + openerUrl = null + openerOpts = null + OUTPUT.length = 0 + npm.config.set('browser', true) + npm.config.set('json', false) + }) + openUrl('https://www.npmjs.com', 'npm home', (err) => { + if (err) + throw err + + t.equal(openerUrl, null, 'did not open') + t.same(openerOpts, null, 'did not open') + t.equal(OUTPUT.length, 1, 'got one logged message') + t.equal(OUTPUT[0].length, 1, 'logged message had one value') + t.matchSnapshot(OUTPUT[0][0], 'printed expected message') + t.done() + }) +}) + +test('prints where to go when given browser does not exist', (t) => { + npm.config.set('browser', 'firefox') + openerResult = Object.assign(new Error('failed'), { code: 'ENOENT' }) + t.teardown(() => { + openerUrl = null + openerOpts = null + OUTPUT.length = 0 + npm.config.set('browser', true) + }) + openUrl('https://www.npmjs.com', 'npm home', (err) => { + if (err) + throw err + + t.equal(openerUrl, 'https://www.npmjs.com', 'tried to open the correct url') + t.same(openerOpts, { command: 'firefox' }, 'tried to use the correct browser') + t.equal(OUTPUT.length, 1, 'got one logged message') + t.equal(OUTPUT[0].length, 1, 'logged message had one value') + t.matchSnapshot(OUTPUT[0][0], 'printed expected message') + t.done() + }) +}) diff --git a/test/lib/utils/otplease.js b/test/lib/utils/otplease.js new file mode 100644 index 000000000..048856b48 --- /dev/null +++ b/test/lib/utils/otplease.js @@ -0,0 +1,94 @@ +const { test } = require('tap') +const requireInject = require('require-inject') + +const readUserInfo = { + otp: async () => '1234', +} + +const otplease = requireInject('../../../lib/utils/otplease.js', { + '../../../lib/utils/read-user-info.js': readUserInfo, +}) + +test('prompts for otp for EOTP', async (t) => { + const stdinTTY = process.stdin.isTTY + const stdoutTTY = process.stdout.isTTY + process.stdin.isTTY = true + process.stdout.isTTY = true + t.teardown(() => { + process.stdin.isTTY = stdinTTY + process.stdout.isTTY = stdoutTTY + }) + + let runs = 0 + const fn = async (opts) => { + if (++runs === 1) + throw Object.assign(new Error('nope'), { code: 'EOTP' }) + + t.equal(opts.some, 'prop', 'carried original options') + t.equal(opts.otp, '1234', 'received the otp') + t.done() + } + + await otplease({ some: 'prop' }, fn) +}) + +test('prompts for otp for 401', async (t) => { + const stdinTTY = process.stdin.isTTY + const stdoutTTY = process.stdout.isTTY + process.stdin.isTTY = true + process.stdout.isTTY = true + t.teardown(() => { + process.stdin.isTTY = stdinTTY + process.stdout.isTTY = stdoutTTY + }) + + let runs = 0 + const fn = async (opts) => { + if (++runs === 1) { + throw Object.assign(new Error('nope'), { + code: 'E401', + body: 'one-time pass required', + }) + } + + t.equal(opts.some, 'prop', 'carried original options') + t.equal(opts.otp, '1234', 'received the otp') + t.done() + } + + await otplease({ some: 'prop' }, fn) +}) + +test('does not prompt for non-otp errors', async (t) => { + const stdinTTY = process.stdin.isTTY + const stdoutTTY = process.stdout.isTTY + process.stdin.isTTY = true + process.stdout.isTTY = true + t.teardown(() => { + process.stdin.isTTY = stdinTTY + process.stdout.isTTY = stdoutTTY + }) + + const fn = async (opts) => { + throw new Error('nope') + } + + t.rejects(otplease({ some: 'prop' }, fn), { message: 'nope' }, 'rejects with the original error') +}) + +test('does not prompt if stdin or stdout is not a tty', async (t) => { + const stdinTTY = process.stdin.isTTY + const stdoutTTY = process.stdout.isTTY + process.stdin.isTTY = false + process.stdout.isTTY = false + t.teardown(() => { + process.stdin.isTTY = stdinTTY + process.stdout.isTTY = stdoutTTY + }) + + const fn = async (opts) => { + throw Object.assign(new Error('nope'), { code: 'EOTP' }) + } + + t.rejects(otplease({ some: 'prop' }, fn), { message: 'nope' }, 'rejects with the original error') +}) diff --git a/test/lib/utils/pulse-till-done.js b/test/lib/utils/pulse-till-done.js new file mode 100644 index 000000000..16c2d521d --- /dev/null +++ b/test/lib/utils/pulse-till-done.js @@ -0,0 +1,35 @@ +const { test } = require('tap') +const requireInject = require('require-inject') + +let pulseStarted = null +const npmlog = { + gauge: { + pulse: () => { + if (pulseStarted) + pulseStarted() + }, + }, +} + +const pulseTillDone = requireInject('../../../lib/utils/pulse-till-done.js', { + npmlog, +}) + +test('pulses (with promise)', async (t) => { + t.teardown(() => { + pulseStarted = null + }) + + let resolver + const promise = new Promise(resolve => { + resolver = resolve + }) + + const result = pulseTillDone.withPromise(promise) + // wait until the gauge has fired at least once + await new Promise(resolve => { + pulseStarted = resolve + }) + resolver('value') + t.resolveMatch(result, 'value', 'returned the resolved promise') +}) diff --git a/test/lib/utils/read-user-info.js b/test/lib/utils/read-user-info.js new file mode 100644 index 000000000..99d85d66c --- /dev/null +++ b/test/lib/utils/read-user-info.js @@ -0,0 +1,116 @@ +const { test } = require('tap') +const requireInject = require('require-inject') + +let readOpts = null +let readResult = null +const read = (opts, cb) => { + readOpts = opts + return cb(null, readResult) +} + +const npmlog = { + clearProgress: () => {}, + showProgress: () => {}, +} + +const npmUserValidate = { + username: (username) => { + if (username === 'invalid') + return new Error('invalid username') + + return null + }, + email: (email) => { + if (email.startsWith('invalid')) + return new Error('invalid email') + + return null + }, +} + +const readUserInfo = requireInject('../../../lib/utils/read-user-info.js', { + read, + npmlog, + 'npm-user-validate': npmUserValidate, +}) + +test('otp', async (t) => { + readResult = '1234' + t.teardown(() => { + readResult = null + readOpts = null + }) + const result = await readUserInfo.otp() + t.equal(result, '1234', 'received the otp') +}) + +test('password', async (t) => { + readResult = 'password' + t.teardown(() => { + readResult = null + readOpts = null + }) + const result = await readUserInfo.password() + t.equal(result, 'password', 'received the password') + t.match(readOpts, { + silent: true, + }, 'got the correct options') +}) + +test('username', async (t) => { + readResult = 'username' + t.teardown(() => { + readResult = null + readOpts = null + }) + const result = await readUserInfo.username() + t.equal(result, 'username', 'received the username') +}) + +test('username - invalid warns and retries', async (t) => { + readResult = 'invalid' + t.teardown(() => { + readResult = null + readOpts = null + }) + + let logMsg + const log = { + warn: (msg) => logMsg = msg, + } + const pResult = readUserInfo.username(null, null, { log }) + // have to swap it to a valid username after execution starts + // or it will loop forever + readResult = 'valid' + const result = await pResult + t.equal(result, 'valid', 'received the username') + t.equal(logMsg, 'invalid username') +}) + +test('email', async (t) => { + readResult = 'foo@bar.baz' + t.teardown(() => { + readResult = null + readOpts = null + }) + const result = await readUserInfo.email() + t.equal(result, 'foo@bar.baz', 'received the email') +}) + +test('email - invalid warns and retries', async (t) => { + readResult = 'invalid@bar.baz' + t.teardown(() => { + readResult = null + readOpts = null + }) + + let logMsg + const log = { + warn: (msg) => logMsg = msg, + } + const pResult = readUserInfo.email(null, null, { log }) + readResult = 'foo@bar.baz' + const result = await pResult + t.equal(result, 'foo@bar.baz', 'received the email') + t.equal(logMsg, 'invalid email') +}) |