Address PR review feedback

packages/kbn-config-schema/README.md

@@ -128,10 +128,10 @@ Validates input data as a number.
 __Output type:__ `number`
 
 __Options:__
-  * `defaultValue: number | Reference<number> | (() => number)` - defines a default value, see [Default values](#default-values) section for more details.
-  * `validate: (value: number) => string | void` - defines a custom validator function, see [Custom validation](#custom-validation) section for more details.
-  * `min: number` - defines a minimum value the number should have.
-  * `max: number` - defines a maximum value the number should have.
+  * `defaultValue: number | string | Reference<number | string> | (() => number | string)` - defines a default value, see [Default values](#default-values) section for more details.
+  * `validate: (value: number | string) => string | void` - defines a custom validator function, see [Custom validation](#custom-validation) section for more details.
+  * `min: number | string` - defines a minimum value the number should have.
+  * `max: number | string` - defines a maximum value the number should have.
 
 __Usage:__
 ```typescript
@@ -148,14 +148,17 @@ Validates input data as a boolean.
 __Output type:__ `boolean`
 
 __Options:__
-  * `defaultValue: boolean | Reference<boolean> | (() => boolean)` - defines a default value, see [Default values](#default-values) section for more details.
-  * `validate: (value: boolean) => string | void` - defines a custom validator function, see [Custom validation](#custom-validation) section for more details.
+  * `defaultValue: boolean | string | Reference<boolean | string> | (() => boolean | string)` - defines a default value, see [Default values](#default-values) section for more details.
+  * `validate: (value: boolean | string) => string | void` - defines a custom validator function, see [Custom validation](#custom-validation) section for more details.
 
 __Usage:__
 ```typescript
 const valueSchema = schema.boolean({ defaultValue: false });
 ```
 
+__Notes:__
+* The `schema.boolean()` also supports a string as input if it equals `'true'` or `'false'` (case-insensitive).
+
 #### `schema.literal()`
 
 Validates input data as a [string](https://www.typescriptlang.org/docs/handbook/advanced-types.html#string-literal-types), [numeric](https://www.typescriptlang.org/docs/handbook/advanced-types.html#numeric-literal-types) or boolean literal.
@@ -397,7 +400,7 @@ const valueSchema = schema.byteSize({ min: '3kb' });
 ```
 
 __Notes:__
-* The string value for `schema.byteSize()` and its options supports the following prefixes: `b`, `kb`, `mb`, `gb` and `tb`.
+* The string value for `schema.byteSize()` and its options supports the following optional suffixes: `b`, `kb`, `mb`, `gb` and `tb`. The default suffix is `b`.
 * The number value is treated as a number of bytes and hence should be a positive integer, e.g. `100` is equal to `'100b'`.
 * Currently you cannot specify zero bytes with a string format and should use number `0` instead.
 
@@ -417,7 +420,7 @@ const valueSchema = schema.duration({ defaultValue: '70ms' });
 ```
 
 __Notes:__
-* The string value for `schema.duration()` supports the following prefixes: `ms`, `s`, `m`, `h`, `d`, `w`, `M` and `Y`.
+* The string value for `schema.duration()` supports the following optional suffixes: `ms`, `s`, `m`, `h`, `d`, `w`, `M` and `Y`. The default suffix is `ms`.
 * The number value is treated as a number of milliseconds and hence should be a positive integer, e.g. `100` is equal to `'100ms'`.
 
 #### `schema.conditional()`

packages/kbn-config-schema/src/byte_size_value/index.test.ts

@@ -42,7 +42,7 @@ describe('parsing units', () => {
   });
 
   test('throws an error when unsupported unit specified', () => {
-    expect(() => ByteSizeValue.parse('1tb')).toThrowError('could not parse byte size value');
+    expect(() => ByteSizeValue.parse('1tb')).toThrowErrorMatchingSnapshot();
   });
 });
 

packages/kbn-config-schema/src/byte_size_value/index.ts

@@ -38,7 +38,8 @@ export class ByteSizeValue {
       const number = Number(text);
       if (typeof number !== 'number' || isNaN(number)) {
         throw new Error(
-          `could not parse byte size value [${text}]. Value must be a safe positive integer.`
+          `Failed to parse [${text}] as byte value. Value must be either number of bytes, or follow the format <count>[b|kb|mb|gb] ` +
+            `(e.g., '1024kb', '200mb', '1gb'), where the number is a safe positive integer.`
         );
       }
       return new ByteSizeValue(number);
@@ -53,8 +54,7 @@ export class ByteSizeValue {
   constructor(private readonly valueInBytes: number) {
     if (!Number.isSafeInteger(valueInBytes) || valueInBytes < 0) {
       throw new Error(
-        `Value in bytes is expected to be a safe positive integer, ` +
-          `but provided [${valueInBytes}]`
+        `Value in bytes is expected to be a safe positive integer, but provided [${valueInBytes}].`
       );
     }
   }

packages/kbn-config-schema/src/duration/index.ts

@@ -28,8 +28,8 @@ function stringToDuration(text: string) {
     const number = Number(text);
     if (typeof number !== 'number' || isNaN(number)) {
       throw new Error(
-        `Failed to parse [${text}] as time value. ` +
-          `Format must be <count>[ms|s|m|h|d|w|M|Y] (e.g. '70ms', '5s', '3d', '1Y')`
+        `Failed to parse [${text}] as time value. Value must be a duration in milliseconds, or follow the format ` +
+          `<count>[ms|s|m|h|d|w|M|Y] (e.g. '70ms', '5s', '3d', '1Y'), where the duration is a safe positive integer.`
       );
     }
     return numberToDuration(number);
@@ -44,8 +44,7 @@ function stringToDuration(text: string) {
 function numberToDuration(numberMs: number) {
   if (!Number.isSafeInteger(numberMs) || numberMs < 0) {
     throw new Error(
-      `Failed to parse [${numberMs}] as time value. ` +
-        `Value should be a safe positive integer number.`
+      `Value in milliseconds is expected to be a safe positive integer, but provided [${numberMs}].`
     );
   }
 

packages/kbn-config-schema/src/types/boolean_type.test.ts

@@ -26,8 +26,12 @@ test('returns value by default', () => {
 test('handles boolean strings', () => {
   expect(schema.boolean().validate('true')).toBe(true);
   expect(schema.boolean().validate('TRUE')).toBe(true);
+  expect(schema.boolean().validate('True')).toBe(true);
+  expect(schema.boolean().validate('TrUe')).toBe(true);
   expect(schema.boolean().validate('false')).toBe(false);
   expect(schema.boolean().validate('FALSE')).toBe(false);
+  expect(schema.boolean().validate('False')).toBe(false);
+  expect(schema.boolean().validate('FaLse')).toBe(false);
 });
 
 test('is required by default', () => {

packages/kbn-config-schema/src/types/byte_size_type.test.ts

@@ -23,7 +23,15 @@ import { ByteSizeValue } from '../byte_size_value';
 const { byteSize } = schema;
 
 test('returns value by default', () => {
-  expect(byteSize().validate('123b')).toMatchSnapshot();
+  expect(byteSize().validate('123b')).toEqual(new ByteSizeValue(123));
+});
+
+test('handles numeric strings', () => {
+  expect(byteSize().validate('123')).toEqual(new ByteSizeValue(123));
+});
+
+test('handles numbers', () => {
+  expect(byteSize().validate(123)).toEqual(new ByteSizeValue(123));
 });
 
 test('is required by default', () => {
@@ -96,7 +104,7 @@ describe('#max', () => {
   });
 });
 
-test('returns error when not string or positive safe integer', () => {
+test('returns error when not valid string or positive safe integer', () => {
   expect(() => byteSize().validate(-123)).toThrowErrorMatchingSnapshot();
 
   expect(() => byteSize().validate(NaN)).toThrowErrorMatchingSnapshot();
@@ -108,4 +116,8 @@ test('returns error when not string or positive safe integer', () => {
   expect(() => byteSize().validate([1, 2, 3])).toThrowErrorMatchingSnapshot();
 
   expect(() => byteSize().validate(/abc/)).toThrowErrorMatchingSnapshot();
+
+  expect(() => byteSize().validate('123foo')).toThrowErrorMatchingSnapshot();
+
+  expect(() => byteSize().validate('123 456')).toThrowErrorMatchingSnapshot();
 });

packages/kbn-config-schema/src/types/duration_type.test.ts

@@ -23,7 +23,15 @@ import { schema } from '..';
 const { duration, object, contextRef, siblingRef } = schema;
 
 test('returns value by default', () => {
-  expect(duration().validate('123s')).toMatchSnapshot();
+  expect(duration().validate('123s')).toEqual(momentDuration(123000));
+});
+
+test('handles numeric string', () => {
+  expect(duration().validate('123000')).toEqual(momentDuration(123000));
+});
+
+test('handles number', () => {
+  expect(duration().validate(123000)).toEqual(momentDuration(123000));
 });
 
 test('is required by default', () => {
@@ -132,7 +140,7 @@ Object {
   });
 });
 
-test('returns error when not string or non-safe positive integer', () => {
+test('returns error when not valid string or non-safe positive integer', () => {
   expect(() => duration().validate(-123)).toThrowErrorMatchingSnapshot();
 
   expect(() => duration().validate(NaN)).toThrowErrorMatchingSnapshot();
@@ -146,4 +154,6 @@ test('returns error when not string or non-safe positive integer', () => {
   expect(() => duration().validate(/abc/)).toThrowErrorMatchingSnapshot();
 
   expect(() => duration().validate('123foo')).toThrowErrorMatchingSnapshot();
+
+  expect(() => duration().validate('123 456')).toThrowErrorMatchingSnapshot();
 });