Improve test stability (#25120)

* improve overview test

* Add custom waiter to maybe-query-record

* add custom waiter to console/ui-panel

* Add flash message check for better visibility into flakiness

* trying to find what's wrong with PKI

* create role happy path uses root token

* make all policy names on pki workflow unique

* some secret test cleanup, not the main offenders

* remove uncessary settled

* Update kv-run-commands.js

* Update kv-run-commands.js

* Update kv-data-fields-test.js

* some missed fixes that were outside the original cherry pick

* remove overview test things

* move testWaiter to logAndOutput command

* nope not working

---------

Co-authored-by: Chelsea Shaw <cshaw@hashicorp.com>

ui/app/components/console/ui-panel.js

@@ -10,6 +10,7 @@ import { getOwner } from '@ember/application';
 import { schedule } from '@ember/runloop';
 import { camelize } from '@ember/string';
 import { task } from 'ember-concurrency';
+import { buildWaiter } from '@ember/test-waiters';
 import ControlGroupError from 'vault/lib/control-group-error';
 import {
   parseCommand,
@@ -21,6 +22,8 @@ import {
   extractDataFromStrings,
 } from 'vault/lib/console-helpers';
 
+const waiter = buildWaiter('web-repl');
+
 export default Component.extend({
   console: service(),
   router: service(),
@@ -49,6 +52,7 @@ export default Component.extend({
 
   executeCommand: task(function* (command, shouldThrow = false) {
     this.set('inputValue', '');
+    const waiterToken = waiter.beginAsync();
     const service = this.console;
     let serviceArgs;
 
@@ -61,6 +65,7 @@ export default Component.extend({
         refresh: () => this.refreshRoute.perform(),
       })
     ) {
+      waiter.endAsync(waiterToken);
       return;
     }
 
@@ -71,6 +76,7 @@ export default Component.extend({
       if (shouldThrow) {
         this.logAndOutput(command, { type: 'help' });
       }
+      waiter.endAsync(waiterToken);
       return;
     }
 
@@ -81,6 +87,7 @@ export default Component.extend({
     const inputError = formattedErrorFromInput(path, method, flags, dataArray);
     if (inputError) {
       this.logAndOutput(command, inputError);
+      waiter.endAsync(waiterToken);
       return;
     }
     try {
@@ -88,10 +95,12 @@ export default Component.extend({
       this.logAndOutput(command, logFromResponse(resp, path, method, flags));
     } catch (error) {
       if (error instanceof ControlGroupError) {
+        waiter.endAsync(waiterToken);
         return this.logAndOutput(command, this.controlGroup.logFromError(error));
       }
       this.logAndOutput(command, logFromError(error, path, method));
     }
+    waiter.endAsync(waiterToken);
   }),
 
   refreshRoute: task(function* () {

ui/app/macros/maybe-query-record.js

@@ -7,6 +7,7 @@ import { computed } from '@ember/object';
 import ObjectProxy from '@ember/object/proxy';
 import PromiseProxyMixin from '@ember/object/promise-proxy-mixin';
 import { resolve } from 'rsvp';
+import { buildWaiter } from '@ember/test-waiters';
 /**
  * after upgrading to Ember 4.12 a secrets test was erroring with "Cannot create a new tag for `<model::capabilities:undefined>` after it has been destroyed"
  * see this GH issue for information on the fix https://github.com/emberjs/ember.js/issues/16541#issuecomment-382403523
@@ -25,14 +26,19 @@ ObjectProxy.reopen({
   },
 });
 
+const waiter = buildWaiter('capabilities');
+
 export function maybeQueryRecord(modelName, options = {}, ...keys) {
   return computed(...keys, 'store', {
     get() {
+      const waiterToken = waiter.beginAsync();
       const query = typeof options === 'function' ? options(this) : options;
       const PromiseObject = ObjectProxy.extend(PromiseProxyMixin);
 
       return PromiseObject.create({
-        promise: query ? this.store.queryRecord(modelName, query) : resolve({}),
+        promise: query
+          ? this.store.queryRecord(modelName, query).finally(() => waiter.endAsync(waiterToken))
+          : resolve({}).finally(() => waiter.endAsync(waiterToken)),
       });
     },
   });

ui/tests/acceptance/auth-test.js

@@ -69,22 +69,25 @@ module('Acceptance | auth', function (hooks) {
       }
       await component.login();
       const lastRequest = this.server.passthroughRequests[this.server.passthroughRequests.length - 1];
-      let body = JSON.parse(lastRequest.requestBody);
-      // Note: x-vault-token used to be lowercase prior to upgrade
+      const body = JSON.parse(lastRequest.requestBody);
+
+      let keys;
+      let included;
       if (backend.type === 'token') {
-        assert.ok(
-          Object.keys(lastRequest.requestHeaders).includes('X-Vault-Token'),
-          'token uses vault token header'
-        );
+        keys = lastRequest.requestHeaders;
+        included = 'X-Vault-Token';
       } else if (backend.type === 'github') {
-        assert.ok(Object.keys(body).includes('token'), 'GitHub includes token');
+        keys = body;
+        included = 'token';
       } else if (backend.type === 'jwt' || backend.type === 'oidc') {
         const authReq = this.server.passthroughRequests[this.server.passthroughRequests.length - 2];
-        body = JSON.parse(authReq.requestBody);
-        assert.ok(Object.keys(body).includes('role'), `${backend.type} includes role`);
+        keys = JSON.parse(authReq.requestBody);
+        included = 'role';
       } else {
-        assert.ok(Object.keys(body).includes('password'), `${backend.type} includes password`);
+        keys = body;
+        included = 'password';
       }
+      assert.ok(Object.keys(keys).includes(included), `${backend.type} includes ${included}`);
     }
   });
 

ui/tests/acceptance/pki/pki-engine-workflow-test.js

@@ -14,8 +14,11 @@ import { click, currentURL, fillIn, find, isSettled, visit } from '@ember/test-h
 import { SELECTORS } from 'vault/tests/helpers/pki/workflow';
 import { adminPolicy, readerPolicy, updatePolicy } from 'vault/tests/helpers/policy-generator/pki';
 import { tokenWithPolicy, runCommands, clearRecords } from 'vault/tests/helpers/pki/pki-run-commands';
+import { runCmd, tokenWithPolicyCmd } from 'vault/tests/helpers/commands';
 import { unsupportedPem } from 'vault/tests/helpers/pki/values';
-
+import { create } from 'ember-cli-page-object';
+import flashMessage from 'vault/tests/pages/components/flash-message';
+const flash = create(flashMessage);
 /**
  * This test module should test the PKI workflow, including:
  * - link between pages and confirm that the url is as expected
@@ -103,9 +106,13 @@ module('Acceptance | pki workflow', function (hooks) {
       const pki_admin_policy = adminPolicy(this.mountPath, 'roles');
       const pki_reader_policy = readerPolicy(this.mountPath, 'roles');
       const pki_editor_policy = updatePolicy(this.mountPath, 'roles');
-      this.pkiRoleReader = await tokenWithPolicy(`pki-reader-${this.mountPath}`, pki_reader_policy);
-      this.pkiRoleEditor = await tokenWithPolicy(`pki-editor-${this.mountPath}`, pki_editor_policy);
-      this.pkiAdminToken = await tokenWithPolicy(`pki-admin-${this.mountPath}`, pki_admin_policy);
+      this.pkiRoleReader = await runCmd(
+        tokenWithPolicyCmd(`pki-reader-${this.mountPath}`, pki_reader_policy)
+      );
+      this.pkiRoleEditor = await runCmd(
+        tokenWithPolicyCmd(`pki-editor-${this.mountPath}`, pki_editor_policy)
+      );
+      this.pkiAdminToken = await runCmd(tokenWithPolicyCmd(`pki-admin-${this.mountPath}`, pki_admin_policy));
       await logout.visit();
       clearRecords(this.store);
     });
@@ -209,7 +216,7 @@ module('Acceptance | pki workflow', function (hooks) {
       await authPage.login(this.pkiAdminToken);
       await visit(`/vault/secrets/${this.mountPath}/pki/overview`);
       assert.strictEqual(currentURL(), `/vault/secrets/${this.mountPath}/pki/overview`);
-      assert.dom(SELECTORS.rolesTab).exists('Roles tab is present');
+      assert.dom(SELECTORS.emptyState).doesNotExist();
       await click(SELECTORS.rolesTab);
       assert.strictEqual(currentURL(), `/vault/secrets/${this.mountPath}/pki/roles`);
       await click(SELECTORS.createRoleLink);
@@ -220,7 +227,11 @@ module('Acceptance | pki workflow', function (hooks) {
 
       await fillIn(SELECTORS.roleForm.roleName, roleName);
       await click(SELECTORS.roleForm.roleCreateButton);
-
+      assert.strictEqual(
+        flash.latestMessage,
+        `Successfully created the role ${roleName}.`,
+        'renders success flash upon creation'
+      );
       assert.strictEqual(currentURL(), `/vault/secrets/${this.mountPath}/pki/roles/${roleName}/details`);
       assert.dom(SELECTORS.breadcrumbs).exists({ count: 4 }, 'Shows 4 breadcrumbs');
       assert.dom(SELECTORS.pageTitle).hasText(`PKI Role ${roleName}`);
@@ -507,9 +518,8 @@ module('Acceptance | pki workflow', function (hooks) {
       ${adminPolicy(this.mountPath)}
       ${readerPolicy(this.mountPath, 'config/cluster')}
       `;
-      this.mixedConfigCapabilities = await tokenWithPolicy(
-        `pki-reader-${this.mountPath}`,
-        mixed_config_policy
+      this.mixedConfigCapabilities = await runCmd(
+        tokenWithPolicyCmd(`pki-reader-${this.mountPath}`, mixed_config_policy)
       );
       await logout.visit();
     });

ui/tests/acceptance/policies/index-test.js

@@ -54,6 +54,8 @@ module('Acceptance | policies/acl', function (hooks) {
 
   test('it navigates to show when clicking on the link', async function (assert) {
     await visit('/vault/policies/acl');
+    await fillIn(SELECT.filterBar, 'default');
+    await waitFor(SELECT.policyByName('default'));
     await click(SELECT.policyByName('default'));
     assert.strictEqual(currentRouteName(), 'vault.cluster.policy.show');
     assert.strictEqual(currentURL(), '/vault/policy/acl/default');
@@ -65,6 +67,8 @@ module('Acceptance | policies/acl', function (hooks) {
     await runCmd(`write sys/policies/acl/${policyName} policy=${window.btoa(POLICY)}`);
     await settled();
     await visit('/vault/policies/acl');
+    await fillIn(SELECT.filterBar, policyName);
+    await waitFor(SELECT.policyByName(policyName));
     assert.dom(SELECT.policyByName(policyName)).exists('policy is shown in list');
     await click(`${SELECT.policyByName(policyName)} [data-test-popup-menu-trigger]`);
     await click(SELECT.delete);
@@ -116,6 +120,7 @@ module('Acceptance | policies/acl', function (hooks) {
       'navigates to policy show on successful save'
     );
     assert.dom(SELECT.policyTitle).hasText(policyLower, 'displays the policy name on the show page');
+    // will fail if you have a license about to expire.
     assert.dom('[data-test-flash-message].is-info').doesNotExist('no flash message is displayed on save');
     await click(SELECT.listBreadcrumb);
 

ui/tests/acceptance/secrets/backend/kv/secret-test.js

@@ -4,7 +4,6 @@
  */
 
 import { click, visit, settled, currentURL, currentRouteName, fillIn } from '@ember/test-helpers';
-import { create } from 'ember-cli-page-object';
 import { module, test } from 'qunit';
 import { setupApplicationTest } from 'ember-qunit';
 import { v4 as uuidv4 } from 'uuid';
@@ -16,17 +15,15 @@ import listPage from 'vault/tests/pages/secrets/backend/list';
 import mountSecrets from 'vault/tests/pages/settings/mount-secret-backend';
 import authPage from 'vault/tests/pages/auth';
 import logout from 'vault/tests/pages/logout';
-import consoleClass from 'vault/tests/pages/components/console/ui-panel';
 import { writeSecret, writeVersionedSecret } from 'vault/tests/helpers/kv/kv-run-commands';
+import { runCmd } from 'vault/tests/helpers/commands';
 import { PAGE } from 'vault/tests/helpers/kv/kv-selectors';
 
-const consoleComponent = create(consoleClass);
-
 const deleteEngine = async function (enginePath, assert) {
   await logout.visit();
   await authPage.login();
-  await consoleComponent.runCommands([`delete sys/mounts/${enginePath}`]);
-  const response = consoleComponent.lastLogOutput;
+
+  const response = await runCmd([`delete sys/mounts/${enginePath}`]);
   assert.strictEqual(
     response,
     `Success! Data deleted (if it existed) at: sys/mounts/${enginePath}`,
@@ -76,11 +73,7 @@ module('Acceptance | secrets/secret/create, read, delete', function (hooks) {
     // https://github.com/hashicorp/vault/issues/5994
     test('v1 key named keys', async function (assert) {
       assert.expect(2);
-      await consoleComponent.runCommands([
-        'vault write sys/mounts/test type=kv',
-        'refresh',
-        'vault write test/a keys=a keys=b',
-      ]);
+      await runCmd(['vault write sys/mounts/test type=kv', 'refresh', 'vault write test/a keys=a keys=b']);
       await showPage.visit({ backend: 'test', id: 'a' });
       assert.ok(showPage.editIsPresent, 'renders the page properly');
       await deleteEngine('test', assert);
@@ -90,16 +83,16 @@ module('Acceptance | secrets/secret/create, read, delete', function (hooks) {
   module('kv v2', function (hooks) {
     hooks.beforeEach(async function () {
       this.backend = `kvv2-${this.uid}`;
-      await consoleComponent.runCommands([`write sys/mounts/${this.backend} type=kv options=version=2`]);
+      await runCmd([`write sys/mounts/${this.backend} type=kv options=version=2`]);
     });
     hooks.afterEach(async function () {
-      await consoleComponent.runCommands([`delete sys/mounts/${this.backend}`]);
+      await runCmd([`delete sys/mounts/${this.backend}`]);
     });
     test('it can create a secret when check-and-set is required', async function (assert) {
       const secretPath = 'foo/bar';
-      await consoleComponent.runCommands(`write ${this.backend}/config cas_required=true`);
+      const output = await runCmd(`write ${this.backend}/config cas_required=true`);
       assert.strictEqual(
-        consoleComponent.lastLogOutput,
+        output,
         `Success! Data written to: ${this.backend}/config`,
         'Engine successfully updated'
       );
@@ -142,7 +135,7 @@ module('Acceptance | secrets/secret/create, read, delete', function (hooks) {
       await mountSecrets.path(this.backend).toggleOptions().version(1).submit();
     });
     hooks.afterEach(async function () {
-      await consoleComponent.runCommands([`delete sys/mounts/${this.backend}`]);
+      await runCmd([`delete sys/mounts/${this.backend}`]);
     });
     test('version 1 performs the correct capabilities lookup', async function (assert) {
       // TODO: while this should pass it doesn't really do anything anymore for us as v1 and v2 are completely separate.
@@ -241,7 +234,7 @@ module('Acceptance | secrets/secret/create, read, delete', function (hooks) {
       assert.expect(paths.length * 2 + 1);
       const secretPath = '2';
       const commands = paths.map((path) => `write '${backend}/${path}/${secretPath}' 3=4`);
-      await consoleComponent.runCommands([...commands, 'refresh']);
+      await runCmd([...commands, 'refresh']);
       for (const path of paths) {
         await listPage.visit({ backend, id: path });
         assert.ok(listPage.secrets.filterBy('text', '2')[0], `${path}: secret is displayed properly`);
@@ -260,8 +253,9 @@ module('Acceptance | secrets/secret/create, read, delete', function (hooks) {
       const secretPath = 'per%cent/%fu ll';
       const [firstPath, secondPath] = secretPath.split('/');
       const commands = [`write '${enginePath}/${secretPath}' 3=4`, `refresh`];
-      await consoleComponent.runCommands(commands);
+      await runCmd(commands);
       await listPage.visitRoot({ backend: enginePath });
+      await settled();
 
       assert.dom(`[data-test-secret-link="${firstPath}/"]`).exists('First section item exists');
       await click(`[data-test-secret-link="${firstPath}/"]`);
@@ -293,7 +287,6 @@ module('Acceptance | secrets/secret/create, read, delete', function (hooks) {
     test('creating a secret with a single or double quote works properly', async function (assert) {
       assert.expect(6);
       const backend = this.backend;
-      // await consoleComponent.runCommands(`write sys/mounts/${backend} type=kv`);
       const paths = ["'some", '"some'];
       for (const path of paths) {
         await listPage.visitRoot({ backend });
@@ -313,7 +306,7 @@ module('Acceptance | secrets/secret/create, read, delete', function (hooks) {
 
     test('filter clears on nav', async function (assert) {
       const backend = this.backend;
-      await consoleComponent.runCommands([
+      await runCmd([
         `vault write sys/mounts/${backend} type=kv`,
         `refresh`,
         `vault write ${backend}/filter/foo keys=a keys=b`,

ui/tests/helpers/kv/kv-run-commands.js

@@ -3,7 +3,8 @@
  * SPDX-License-Identifier: BUSL-1.1
  */
 
-import { click, fillIn, visit } from '@ember/test-helpers';
+/* eslint-disable ember/no-settled-after-test-helper */
+import { click, fillIn, visit, settled } from '@ember/test-helpers';
 import { FORM } from './kv-selectors';
 import { encodePath } from 'vault/utils/path-encoding-helpers';
 
@@ -12,18 +13,22 @@ import { encodePath } from 'vault/utils/path-encoding-helpers';
 export const writeSecret = async function (backend, path, key, val, ns = null) {
   const url = `vault/secrets/${backend}/kv/create`;
   ns ? await visit(url + `?namespace=${ns}`) : await visit(url);
+  await settled();
   await fillIn(FORM.inputByAttr('path'), path);
   await fillIn(FORM.keyInput(), key);
   await fillIn(FORM.maskedValueInput(), val);
-  return click(FORM.saveBtn);
+  await click(FORM.saveBtn);
+  await settled();
+  return;
 };
 
 export const writeVersionedSecret = async function (backend, path, key, val, version = 2, ns = null) {
   await writeSecret(backend, path, 'key-1', 'val-1', ns);
+  await settled();
   for (let currentVersion = 2; currentVersion <= version; currentVersion++) {
     const url = `/vault/secrets/${backend}/kv/${encodeURIComponent(path)}/details/edit`;
     ns ? await visit(url + `?namespace=${ns}`) : await visit(url);
-
+    await settled();
     if (currentVersion === version) {
       await fillIn(FORM.keyInput(), key);
       await fillIn(FORM.maskedValueInput(), val);
@@ -32,6 +37,7 @@ export const writeVersionedSecret = async function (backend, path, key, val, ver
       await fillIn(FORM.maskedValueInput(), `val-${currentVersion}`);
     }
     await click(FORM.saveBtn);
+    await settled();
   }
   return;
 };

ui/tests/pages/auth.js

@@ -29,7 +29,9 @@ export default create({
     window.localStorage.clear();
     await this.visit({ with: 'token' });
     await settled();
-    return this.tokenInput(token).submit();
+    await this.tokenInput(token).submit();
+    await settled();
+    return;
   },
   loginUsername: async function (username, password, path) {
     // make sure we're always logged out and logged back in
@@ -44,7 +46,8 @@ export default create({
       await this.mountPath(path);
     }
     await this.usernameInput(username);
-    return this.passwordInput(password).submit();
+    await this.passwordInput(password).submit();
+    return;
   },
   loginNs: async function (ns, token = rootToken) {
     // make sure we're always logged out and logged back in
@@ -65,6 +68,7 @@ export default create({
     if (clearNamespace) {
       await this.namespaceInput('');
     }
+    await settled();
     return;
   },
 });

ui/tests/pages/secrets/backend/kv/edit-secret.js

@@ -5,6 +5,7 @@
 
 import { Base } from '../create';
 import { clickable, create, fillable } from 'ember-cli-page-object';
+import { settled } from '@ember/test-helpers';
 
 export default create({
   ...Base,
@@ -14,6 +15,8 @@ export default create({
   save: clickable('[data-test-secret-save]'),
   toggleJSON: clickable('[data-test-toggle-input="json"]'),
   createSecret: async function (path, key, value) {
-    return this.path(path).secretKey(key).secretValue(value).save();
+    await this.path(path).secretKey(key).secretValue(value).save();
+    await settled();
+    return;
   },
 });