Merge branch 'master' into kertal-pr-2020-03-13-remove-StateManagemen…

…tConfigProvider

docs/development/core/server/kibana-plugin-core-server.routeconfig.validate.md

@@ -14,7 +14,7 @@ validate: RouteValidatorFullConfig<P, Q, B> | false;
 
 ## Remarks
 
-You \*must\* specify a validation schema to be able to read: - url path segments - request query - request body To opt out of validating the request, specify `validate: false`<!-- -->. In this case request params, query, and body will be \*\*empty\*\* objects and have no access to raw values. In some cases you may want to use another validation library. To do this, you need to instruct the `@kbn/config-schema` library to output \*\*non-validated values\*\* with setting schema as `schema.object({}, { allowUnknowns: true })`<!-- -->;
+You \*must\* specify a validation schema to be able to read: - url path segments - request query - request body To opt out of validating the request, specify `validate: false`<!-- -->. In this case request params, query, and body will be \*\*empty\*\* objects and have no access to raw values. In some cases you may want to use another validation library. To do this, you need to instruct the `@kbn/config-schema` library to output \*\*non-validated values\*\* with setting schema as `schema.object({}, { unknowns: 'allow' })`<!-- -->;
 
 ## Example
 
@@ -49,7 +49,7 @@ router.get({
   path: 'path/{id}',
   validate: {
     // handler has access to raw non-validated params in runtime
-    params: schema.object({}, { allowUnknowns: true })
+    params: schema.object({}, { unknowns: 'allow' })
   },
 },
 (context, req, res,) {

packages/kbn-config-schema/README.md

@@ -239,7 +239,7 @@ __Output type:__ `{ [K in keyof TProps]: TypeOf<TProps[K]> } as TObject`
 __Options:__
   * `defaultValue: TObject | Reference<TObject> | (() => TObject)` - defines a default value, see [Default values](#default-values) section for more details.
   * `validate: (value: TObject) => string | void` - defines a custom validator function, see [Custom validation](#custom-validation) section for more details.
-  * `allowUnknowns: boolean` - indicates whether unknown object properties should be allowed. It's `false` by default.
+  * `unknowns: 'allow' | 'ignore' | 'forbid'` - indicates whether unknown object properties should be allowed, ignored, or forbidden. It's `forbid` by default.
 
 __Usage:__
 ```typescript
@@ -250,7 +250,7 @@ const valueSchema = schema.object({
 ```
 
 __Notes:__
-* Using `allowUnknowns` is discouraged and should only be used in exceptional circumstances. Consider using `schema.recordOf()` instead.
+* Using `unknowns: 'allow'` is discouraged and should only be used in exceptional circumstances. Consider using `schema.recordOf()` instead.
 * Currently `schema.object()` always has a default value of `{}`, but this may change in the near future. Try to not rely on this behaviour and specify default value explicitly or use `schema.maybe()` if the value is optional.
 * `schema.object()` also supports a json string as input if it can be safely parsed using `JSON.parse` and if the resulting value is a plain object.
 

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

@@ -276,10 +276,10 @@ test('individual keys can validated', () => {
   );
 });
 
-test('allow unknown keys when allowUnknowns = true', () => {
+test('allow unknown keys when unknowns = `allow`', () => {
   const type = schema.object(
     { foo: schema.string({ defaultValue: 'test' }) },
-    { allowUnknowns: true }
+    { unknowns: 'allow' }
   );
 
   expect(
@@ -292,10 +292,10 @@ test('allow unknown keys when allowUnknowns = true', () => {
   });
 });
 
-test('allowUnknowns = true affects only own keys', () => {
+test('unknowns = `allow` affects only own keys', () => {
   const type = schema.object(
     { foo: schema.object({ bar: schema.string() }) },
-    { allowUnknowns: true }
+    { unknowns: 'allow' }
   );
 
   expect(() =>
@@ -308,14 +308,45 @@ test('allowUnknowns = true affects only own keys', () => {
   ).toThrowErrorMatchingInlineSnapshot(`"[foo.baz]: definition for this key is missing"`);
 });
 
-test('does not allow unknown keys when allowUnknowns = false', () => {
+test('does not allow unknown keys when unknowns = `forbid`', () => {
   const type = schema.object(
     { foo: schema.string({ defaultValue: 'test' }) },
-    { allowUnknowns: false }
+    { unknowns: 'forbid' }
   );
   expect(() =>
     type.validate({
       bar: 'baz',
     })
   ).toThrowErrorMatchingInlineSnapshot(`"[bar]: definition for this key is missing"`);
 });
+
+test('allow and remove unknown keys when unknowns = `ignore`', () => {
+  const type = schema.object(
+    { foo: schema.string({ defaultValue: 'test' }) },
+    { unknowns: 'ignore' }
+  );
+
+  expect(
+    type.validate({
+      bar: 'baz',
+    })
+  ).toEqual({
+    foo: 'test',
+  });
+});
+
+test('unknowns = `ignore` affects only own keys', () => {
+  const type = schema.object(
+    { foo: schema.object({ bar: schema.string() }) },
+    { unknowns: 'ignore' }
+  );
+
+  expect(() =>
+    type.validate({
+      foo: {
+        bar: 'bar',
+        baz: 'baz',
+      },
+    })
+  ).toThrowErrorMatchingInlineSnapshot(`"[foo.baz]: definition for this key is missing"`);
+});

packages/kbn-config-schema/src/types/object_type.ts

@@ -30,17 +30,25 @@ export type TypeOf<RT extends Type<any>> = RT['type'];
 // this might not have perfect _rendering_ output, but it will be typed.
 export type ObjectResultType<P extends Props> = Readonly<{ [K in keyof P]: TypeOf<P[K]> }>;
 
+interface UnknownOptions {
+  /**
+   * Options for dealing with unknown keys:
+   * - allow: unknown keys will be permitted
+   * - ignore: unknown keys will not fail validation, but will be stripped out
+   * - forbid (default): unknown keys will fail validation
+   */
+  unknowns?: 'allow' | 'ignore' | 'forbid';
+}
+
 export type ObjectTypeOptions<P extends Props = any> = TypeOptions<
   { [K in keyof P]: TypeOf<P[K]> }
-> & {
-  /** Should uknown keys not be defined in the schema be allowed. Defaults to `false` */
-  allowUnknowns?: boolean;
-};
+> &
+  UnknownOptions;
 
 export class ObjectType<P extends Props = any> extends Type<ObjectResultType<P>> {
   private props: Record<string, AnySchema>;
 
-  constructor(props: P, { allowUnknowns = false, ...typeOptions }: ObjectTypeOptions<P> = {}) {
+  constructor(props: P, { unknowns = 'forbid', ...typeOptions }: ObjectTypeOptions<P> = {}) {
     const schemaKeys = {} as Record<string, AnySchema>;
     for (const [key, value] of Object.entries(props)) {
       schemaKeys[key] = value.getSchema();
@@ -50,7 +58,8 @@ export class ObjectType<P extends Props = any> extends Type<ObjectResultType<P>>
       .keys(schemaKeys)
       .default()
       .optional()
-      .unknown(Boolean(allowUnknowns));
+      .unknown(unknowns === 'allow')
+      .options({ stripUnknown: { objects: unknowns === 'ignore' } });
 
     super(schema, typeOptions);
     this.props = schemaKeys;

packages/kbn-test/src/functional_test_runner/cli.ts

@@ -48,12 +48,15 @@ export function runFtrCli() {
           kbnTestServer: {
             installDir: parseInstallDir(flags),
           },
+          suiteFiles: {
+            include: toArray(flags.include as string | string[]).map(makeAbsolutePath),
+            exclude: toArray(flags.exclude as string | string[]).map(makeAbsolutePath),
+          },
           suiteTags: {
             include: toArray(flags['include-tag'] as string | string[]),
             exclude: toArray(flags['exclude-tag'] as string | string[]),
           },
           updateBaselines: flags.updateBaselines,
-          excludeTestFiles: flags.exclude || undefined,
         }
       );
 
@@ -104,7 +107,15 @@ export function runFtrCli() {
     },
     {
       flags: {
-        string: ['config', 'grep', 'exclude', 'include-tag', 'exclude-tag', 'kibana-install-dir'],
+        string: [
+          'config',
+          'grep',
+          'include',
+          'exclude',
+          'include-tag',
+          'exclude-tag',
+          'kibana-install-dir',
+        ],
         boolean: ['bail', 'invert', 'test-stats', 'updateBaselines', 'throttle', 'headless'],
         default: {
           config: 'test/functional/config.js',
@@ -115,7 +126,8 @@ export function runFtrCli() {
         --bail             stop tests after the first failure
         --grep <pattern>   pattern used to select which tests to run
         --invert           invert grep to exclude tests
-        --exclude=file     path to a test file that should not be loaded
+        --include=file     a test file to be included, pass multiple times for multiple files
+        --exclude=file     a test file to be excluded, pass multiple times for multiple files
         --include-tag=tag  a tag to be included, pass multiple times for multiple tags
         --exclude-tag=tag  a tag to be excluded, pass multiple times for multiple tags
         --test-stats       print the number of tests (included and excluded) to STDERR

packages/kbn-test/src/functional_test_runner/lib/config/schema.ts

@@ -64,9 +64,16 @@ export const schema = Joi.object()
     testFiles: Joi.array().items(Joi.string()),
     testRunner: Joi.func(),
 
-    excludeTestFiles: Joi.array()
-      .items(Joi.string())
-      .default([]),
+    suiteFiles: Joi.object()
+      .keys({
+        include: Joi.array()
+          .items(Joi.string())
+          .default([]),
+        exclude: Joi.array()
+          .items(Joi.string())
+          .default([]),
+      })
+      .default(),
 
     suiteTags: Joi.object()
       .keys({

packages/kbn-test/src/functional_test_runner/lib/mocha/decorate_mocha_ui.js

@@ -16,7 +16,8 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-
+import { relative } from 'path';
+import { REPO_ROOT } from '@kbn/dev-utils';
 import { createAssignmentProxy } from './assignment_proxy';
 import { wrapFunction } from './wrap_function';
 import { wrapRunnableArgs } from './wrap_runnable_args';
@@ -65,6 +66,10 @@ export function decorateMochaUi(lifecycle, context) {
             this._tags = [].concat(this._tags || [], tags);
           };
 
+          const relativeFilePath = relative(REPO_ROOT, this.file);
+          this.tags(relativeFilePath);
+          this.suiteTag = relativeFilePath; // The tag that uniquely targets this suite/file
+
           provider.call(this);
 
           after(async () => {

packages/kbn-test/src/functional_test_runner/lib/mocha/load_test_files.js

@@ -31,28 +31,12 @@ import { decorateMochaUi } from './decorate_mocha_ui';
  *  @param  {String} path
  *  @return {undefined} - mutates mocha, no return value
  */
-export const loadTestFiles = ({
-  mocha,
-  log,
-  lifecycle,
-  providers,
-  paths,
-  excludePaths,
-  updateBaselines,
-}) => {
-  const pendingExcludes = new Set(excludePaths.slice(0));
-
+export const loadTestFiles = ({ mocha, log, lifecycle, providers, paths, updateBaselines }) => {
   const innerLoadTestFile = path => {
     if (typeof path !== 'string' || !isAbsolute(path)) {
       throw new TypeError('loadTestFile() only accepts absolute paths');
     }
 
-    if (pendingExcludes.has(path)) {
-      pendingExcludes.delete(path);
-      log.warning('Skipping test file %s', path);
-      return;
-    }
-
     loadTracer(path, `testFile[${path}]`, () => {
       log.verbose('Loading test file %s', path);
 
@@ -94,13 +78,4 @@ export const loadTestFiles = ({
   };
 
   paths.forEach(innerLoadTestFile);
-
-  if (pendingExcludes.size) {
-    throw new Error(
-      `After loading all test files some exclude paths were not consumed:${[
-        '',
-        ...pendingExcludes,
-      ].join('\n  -')}`
-    );
-  }
 };

packages/kbn-test/src/functional_test_runner/lib/mocha/setup_mocha.js

@@ -18,6 +18,8 @@
  */
 
 import Mocha from 'mocha';
+import { relative } from 'path';
+import { REPO_ROOT } from '@kbn/dev-utils';
 
 import { loadTestFiles } from './load_test_files';
 import { filterSuitesByTags } from './filter_suites_by_tags';
@@ -50,10 +52,20 @@ export async function setupMocha(lifecycle, log, config, providers) {
     lifecycle,
     providers,
     paths: config.get('testFiles'),
-    excludePaths: config.get('excludeTestFiles'),
     updateBaselines: config.get('updateBaselines'),
   });
 
+  // Each suite has a tag that is the path relative to the root of the repo
+  // So we just need to take input paths, make them relative to the root, and use them as tags
+  // Also, this is a separate filterSuitesByTags() call so that the test suites will be filtered first by
+  //  files, then by tags. This way, you can target tags (like smoke) in a specific file.
+  filterSuitesByTags({
+    log,
+    mocha,
+    include: config.get('suiteFiles.include').map(file => relative(REPO_ROOT, file)),
+    exclude: config.get('suiteFiles.exclude').map(file => relative(REPO_ROOT, file)),
+  });
+
   filterSuitesByTags({
     log,
     mocha,