From c68c975d883e118f101a8f8c6cb06886b7b00a29 Mon Sep 17 00:00:00 2001 From: Julien Gilli Date: Mon, 3 Apr 2017 18:11:06 -0700 Subject: [PATCH] changes according to latest code review --- CHANGES.md | 3 + lib/common.js | 8 +-- lib/do_volume/do_create.js | 8 +-- lib/do_volume/do_delete.js | 8 ++- lib/volumes.js | 12 ++-- test/unit/parseVolumeSize.test.js | 92 +++++++++++++++++++++++++++++++ 6 files changed, 113 insertions(+), 18 deletions(-) create mode 100644 test/unit/parseVolumeSize.test.js diff --git a/CHANGES.md b/CHANGES.md index a217b05..63987a3 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -19,6 +19,9 @@ Known issues: Use `triton volume --help` to get help on all of these commands. + Note that these commands are hidden for now. They will be made visible by + default once the server-side support for volumes is shipped in Triton. + - [joyent/node-triton#183] `triton profile create` will no longer use ANSI codes for styling if stdout isn't a TTY. diff --git a/lib/common.js b/lib/common.js index 37457f8..b51d47e 100644 --- a/lib/common.js +++ b/lib/common.js @@ -141,16 +141,16 @@ function _parseFilterKeyAndValue(filterComponent, validKeys) { var idx = filterComponent.indexOf('='); if (idx === -1) { - throw new errors.UsageError(format( + throw new errors.UsageError(format( 'invalid filter: "%s" (must be of the form "field=value")', - filterComponent)); + filterComponent)); } var k = filterComponent.slice(0, idx); var v = filterComponent.slice(idx + 1); if (validKeys && validKeys.indexOf(k) === -1) { - throw new errors.UsageError(format( + throw new errors.UsageError(format( 'invalid filter name: "%s" (must be one of "%s")', - k, validKeys.join('", "'))); + k, validKeys.join('", "'))); } return { diff --git a/lib/do_volume/do_create.js b/lib/do_volume/do_create.js index 369c15a..7676670 100644 --- a/lib/do_volume/do_create.js +++ b/lib/do_volume/do_create.js @@ -152,11 +152,9 @@ do_create.options = [ names: ['size', 's'], type: 'string', helpArg: 'SIZE', - help: 'The `size` input parameter must match the following regular ' + - 'expression: /(\d+)(g|m|G|M|gb|mb|GB|MB)/ All units are in ' + - 'ibibytes (mebibytes and gibibytes). `g`, `G`, `gb` and `GB` ' + - 'stand for "gibibytes". `m`, `M`, `mb` and `MB` stand for ' + - '"mebibytes".', + help: 'The size of the volume to create, in the form ' + + '``, e.g. `20G`. must be > 0. Supported ' + + 'units are `G` or `g` for gibibytes and `M` or `m` for mebibytes.', completionType: 'tritonvolumesize' }, { diff --git a/lib/do_volume/do_delete.js b/lib/do_volume/do_delete.js index ad5bf3a..4ac15d6 100644 --- a/lib/do_volume/do_delete.js +++ b/lib/do_volume/do_delete.js @@ -118,13 +118,19 @@ function do_delete(subcmd, opts, args, cb) { function deleteVolumes(ctx, next) { vasync.forEachParallel({ func: function doDeleteVolume(volumeId, done) { + if (opts.wait === undefined) { + console.log('Deleting volume %s...', volumeId); + } + self.top.tritonapi.deleteVolume({ id: volumeId, wait: opts.wait && opts.wait.length > 0, waitTimeout: opts.wait_timeout * 1000 }, function onVolDeleted(volDelErr) { if (!volDelErr) { - console.log('Deleted volume %s', volumeId); + if (opts.wait !== undefined) { + console.log('Deleted volume %s', volumeId); + } } else { console.error('Error when deleting volume %s: %s', volumeId, volDelErr); diff --git a/lib/volumes.js b/lib/volumes.js index d839aac..bd43ca1 100644 --- a/lib/volumes.js +++ b/lib/volumes.js @@ -22,25 +22,21 @@ function throwInvalidSize(size) { * If "size" is not a valid size string, an error is thrown. */ function parseVolumeSize(size) { - assert.optionalString(size, 'size'); + assert.string(size, 'size'); var MIBS_IN_GB = 1024; var MULTIPLIERS_TABLE = { g: MIBS_IN_GB, - GB: MIBS_IN_GB, + G: MIBS_IN_GB, m: 1, - MB: 1 + M: 1 }; var multiplierSymbol, multiplier; var baseValue; - if (size === undefined) { - return undefined; - } - - var matches = size.match(/(\d+)(g|m|G|M|gb|mb|GB|MB)/); + var matches = size.match(/^([1-9]\d*)(g|m|G|M)$/); if (!matches) { throwInvalidSize(size); } diff --git a/test/unit/parseVolumeSize.test.js b/test/unit/parseVolumeSize.test.js new file mode 100644 index 0000000..3e92ac5 --- /dev/null +++ b/test/unit/parseVolumeSize.test.js @@ -0,0 +1,92 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +/* + * Copyright (c) 2017, Joyent, Inc. + */ + +/* + * Unit tests for `parseVolumeSize` used by `triton volume ...`. + */ + +var assert = require('assert-plus'); +var test = require('tape'); + +var parseVolumeSize = require('../../lib/volumes').parseVolumeSize; + +test('parseVolumeSize', function (tt) { + tt.test('parsing invalid sizes', function (t) { + var invalidVolumeSizes = [ + 'foo', + '0', + '-42', + '-42m', + '-42g', + '', + '42Gasdf', + '42gasdf', + '42asdf', + 'asdf42G', + 'asdf42g', + 'asdf42', + '042g', + '042G', + '042', + 0, + 42, + -42, + 42.1, + -42.1, + undefined, + null, + {} + ]; + + invalidVolumeSizes.forEach(function parse(invalidVolumeSize) { + var parseErr; + + try { + parseVolumeSize(invalidVolumeSize); + } catch (err) { + parseErr = err; + } + + t.ok(parseErr, 'parsing invalid volume size: ' + invalidVolumeSize + + ' should throw'); + }); + + t.end(); + }); + + tt.test('parsing valid sizes', function (t) { + var validVolumeSizes = [ + {input: '42g', expectedOutput: 42 * 1024}, + {input: '42G', expectedOutput: 42 * 1024}, + {input: '42m', expectedOutput: 42}, + {input: '42M', expectedOutput: 42} + ]; + + validVolumeSizes.forEach(function parse(validVolumeSize) { + var parseErr; + var volSizeInMebibytes; + + try { + volSizeInMebibytes = parseVolumeSize(validVolumeSize.input); + } catch (err) { + parseErr = err; + } + + t.ifErr(parseErr, 'parsing valid volume size: ' + + validVolumeSize.input + ' should not throw'); + t.equal(validVolumeSize.expectedOutput, volSizeInMebibytes, + 'parsed volume size for "' + validVolumeSize.input + '" ' + + 'should equal to ' + validVolumeSize.expectedOutput + + ' mebibytes'); + }); + + t.end(); + }); +}); \ No newline at end of file