variable get command shows usage error when device param is not an id…

… and --product flag is set
particle-iot · May 6, 2020 · 3fe8885 · 3fe8885
1 parent 686dc0e
commit 3fe8885
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 19 deletions.
diff --git a/src/cli/variable.js b/src/cli/variable.js
@@ -27,9 +27,9 @@ module.exports = ({ commandProcessor, root }) => {
 			return new VariableCommand().getValue(args);
 		},
 		examples: {
-			'$0 $command basement temperature': 'Read the temperature variable from the device basement',
-			'$0 $command basement temperature --product 12345': 'Read the temperature variable from the device basement within product 12345',
-			'$0 $command all temperature': 'Read the temperature variable from all my devices'
+			'$0 $command basement temperature': 'Read the `temperature` variable from the device `basement`',
+			'$0 $command 0123456789abcdef01234567 temperature --product 12345': 'Read the `temperature` variable from the device with id `0123456789abcdef01234567` within product `12345`',
+			'$0 $command all temperature': 'Read the `temperature` variable from all my devices'
 		}
 	});
 

diff --git a/src/cli/variable.test.js b/src/cli/variable.test.js
@@ -91,9 +91,9 @@ describe('Variable Command-Line Interface', () => {
 					'  --product  Target a device within the given Product ID or Slug  [string]',
 					'',
 					'Examples:',
-					'  particle variable get basement temperature                  Read the temperature variable from the device basement',
-					'  particle variable get basement temperature --product 12345  Read the temperature variable from the device basement within product 12345',
-					'  particle variable get all temperature                       Read the temperature variable from all my devices',
+					'  particle variable get basement temperature                                  Read the `temperature` variable from the device `basement`',
+					'  particle variable get 0123456789abcdef01234567 temperature --product 12345  Read the `temperature` variable from the device with id `0123456789abcdef01234567` within product `12345`',
+					'  particle variable get all temperature                                       Read the `temperature` variable from all my devices',
 					''
 				].join(os.EOL));
 			});

diff --git a/src/cmd/variable.js b/src/cmd/variable.js
@@ -33,6 +33,8 @@ module.exports = class VariableCommand extends CLICommandBase {
 				return this.showUsageError(
 					'`device` parameter is required when `--product` flag is set'
 				);
+			} else if (!this.isDeviceId(device)){
+				return this.showProductDeviceNameUsageError(device);
 			}
 
 			if (!variableName){
@@ -98,7 +100,7 @@ module.exports = class VariableCommand extends CLICommandBase {
 					const result = results[i];
 
 					if (result.error){
-						console.log('Error:', result.error);
+						this.ui.stdout.write(`Error: ${result.error}${os.EOL}`);
 						hasErrors = true;
 						continue;
 					}
@@ -112,7 +114,7 @@ module.exports = class VariableCommand extends CLICommandBase {
 					}
 
 					parts.push(result.result);
-					console.log(parts.join(', '));
+					this.ui.stdout.write(`${parts.join(', ')}${os.EOL}`);
 				}
 
 				if (hasErrors){
@@ -138,9 +140,9 @@ module.exports = class VariableCommand extends CLICommandBase {
 			.then(({ deviceIds, variableName }) => {
 				if (delay < settings.minimumApiDelay){
 					delay = settings.minimumApiDelay;
-					console.error(`Delay was too short, resetting to ${settings.minimumApiDelay}ms`);
+					this.ui.stderr.write(`Delay was too short, resetting to ${settings.minimumApiDelay}ms${os.EOL}`);
 				}
-				console.error('Hit CTRL-C to stop!');
+				this.ui.stderr.write(`Hit CTRL-C to stop!${os.EOL}`);
 				return this._pollForVariable(deviceIds, variableName, { delay, time });
 			})
 			.catch(err => {
@@ -215,7 +217,7 @@ module.exports = class VariableCommand extends CLICommandBase {
 			return Promise.resolve(this._cachedVariableList);
 		}
 
-		console.error('polling server to see what devices are online, and what variables are available');
+		this.ui.stderr.write(`polling server to see what devices are online, and what variables are available${os.EOL}`);
 
 		const api = new LegacyApiClient();
 		api.ensureToken();
@@ -224,7 +226,7 @@ module.exports = class VariableCommand extends CLICommandBase {
 			.then(() => api.listDevices())
 			.then(devices => {
 				if (!devices || (devices.length === 0)){
-					console.log('No devices found.');
+					this.ui.stderr.write(`No devices found.${os.EOL}`);
 					this._cachedVariableList = null;
 				} else {
 					const promises = [];

diff --git a/test/e2e/get.e2e.js b/test/e2e/get.e2e.js
@@ -25,9 +25,9 @@ describe('Get Commands [@device]', () => {
 		'  --product  Target a device within the given Product ID or Slug  [string]',
 		'',
 		'Examples:',
-		'  particle get basement temperature                  Read the temperature variable from the device basement',
-		'  particle get basement temperature --product 12345  Read the temperature variable from the device basement within product 12345',
-		'  particle get all temperature                       Read the temperature variable from all my devices'
+		'  particle get basement temperature                                  Read the `temperature` variable from the device `basement`',
+		'  particle get 0123456789abcdef01234567 temperature --product 12345  Read the `temperature` variable from the device with id `0123456789abcdef01234567` within product `12345`',
+		'  particle get all temperature                                       Read the `temperature` variable from all my devices'
 	];
 
 	before(async () => {

diff --git a/test/e2e/variable.e2e.js b/test/e2e/variable.e2e.js
@@ -9,7 +9,8 @@ const {
 	DEVICE_NAME,
 	DEVICE_PLATFORM_NAME,
 	PRODUCT_01_ID,
-	PRODUCT_01_DEVICE_02_ID
+	PRODUCT_01_DEVICE_02_ID,
+	PRODUCT_01_DEVICE_02_NAME
 } = require('../lib/env');
 
 
@@ -118,9 +119,9 @@ describe('Variable Commands [@device]', () => {
 			'  --product  Target a device within the given Product ID or Slug  [string]',
 			'',
 			'Examples:',
-			'  particle get basement temperature                  Read the temperature variable from the device basement',
-			'  particle get basement temperature --product 12345  Read the temperature variable from the device basement within product 12345',
-			'  particle get all temperature                       Read the temperature variable from all my devices'
+			'  particle get basement temperature                                  Read the `temperature` variable from the device `basement`',
+			'  particle get 0123456789abcdef01234567 temperature --product 12345  Read the `temperature` variable from the device with id `0123456789abcdef01234567` within product `12345`',
+			'  particle get all temperature                                       Read the `temperature` variable from all my devices'
 		];
 
 		it('Gets a variable by name', async () => {
@@ -143,6 +144,31 @@ describe('Variable Commands [@device]', () => {
 			expect(exitCode).to.equal(0);
 		});
 
+		it('Uses default when `--delay` is too short', async () => {
+			const args = ['variable', 'monitor', DEVICE_ID, 'version', '--delay', 1];
+			const subprocess = cli.run(args);
+			const received = [];
+
+			await waitForResult(subprocess, (data) => {
+				const log = data.toString('utf8').trim();
+
+				received.push(log);
+
+				if (received.length > 3){
+					return true;
+				}
+				return false;
+			});
+
+			const { isCanceled } = await subprocess;
+			const [alert, msg, ...results] = received;
+
+			expect(alert).to.equal('Delay was too short, resetting to 500ms');
+			expect(msg).to.equal('Hit CTRL-C to stop!');
+			expect(results).to.have.lengthOf.above(1);
+			expect(isCanceled).to.equal(true);
+		});
+
 		// TODO (mirande): need to ensure device is running expected firmware and online
 		// once flashing product devices is implemented - as it is, the expectation
 		// is that your product device is running the `stroby` firmware found in:
@@ -156,6 +182,19 @@ describe('Variable Commands [@device]', () => {
 			expect(exitCode).to.equal(0);
 		});
 
+		it('Fails to get an unknown variable', async () => {
+			const args = ['get', DEVICE_NAME, 'NOPE'];
+			const { stdout, stderr, exitCode } = await cli.run(args);
+			const msg = [
+				'Error: Unknown Variable: NOPE',
+				'Error while reading value: Some variables could not be read'
+			].join(os.EOL);
+
+			expect(stdout).to.include(msg);
+			expect(stderr).to.equal('');
+			expect(exitCode).to.equal(1);
+		});
+
 		it('Fails to get a variable from a product device when `device` param is not provided', async () => {
 			const args = ['get', '--product', PRODUCT_01_ID];
 			const { stdout, stderr, exitCode } = await cli.run(args);
@@ -165,6 +204,15 @@ describe('Variable Commands [@device]', () => {
 			expect(exitCode).to.equal(1);
 		});
 
+		it('Fails to get a variable from a product device when `device` param is not an id', async () => {
+			const args = ['get', PRODUCT_01_DEVICE_02_NAME, 'version', '--product', PRODUCT_01_ID];
+			const { stdout, stderr, exitCode } = await cli.run(args);
+
+			expect(stdout).to.include(`\`device\` must be an id when \`--product\` flag is set - received: ${PRODUCT_01_DEVICE_02_NAME}`);
+			expect(stderr.split(os.EOL)).to.include.members(help);
+			expect(exitCode).to.equal(1);
+		});
+
 		it('Fails to get a variable from a product device when `variableName` param is not provided', async () => {
 			const args = ['get', PRODUCT_01_DEVICE_02_ID, '--product', PRODUCT_01_ID];
 			const { stdout, stderr, exitCode } = await cli.run(args);