changes according to latest code review

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.
 

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 {

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 ' +
+        help: 'The size of the volume to create, in the form ' +
-            'expression: /(\d+)(g|m|G|M|gb|mb|GB|MB)/ All units are in ' +
+            '`<integer><unit>`, e.g. `20G`. <integer> must be > 0. Supported ' +
-            'ibibytes (mebibytes and gibibytes). `g`, `G`, `gb` and `GB` ' +
+            'units are `G` or `g` for gibibytes and `M` or `m` for mebibytes.',
-            'stand for "gibibytes". `m`, `M`, `mb` and `MB` stand for ' +
-            '"mebibytes".',
         completionType: 'tritonvolumesize'
     },
     {

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);

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) {
+    var matches = size.match(/^([1-9]\d*)(g|m|G|M)$/);
-        return undefined;
-    }
-
-    var matches = size.match(/(\d+)(g|m|G|M|gb|mb|GB|MB)/);
     if (!matches) {
         throwInvalidSize(size);
     }

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();
+    });
+});