Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor a11yAudit for auth-method list test #2520

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
8 changes: 7 additions & 1 deletion documentation/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,10 @@ To learn how to add a translation, [check documentation](https://ember-intl.gith

All our translations files are located [in the translations folder](https://github.com/hashicorp/boundary-ui/tree/main/addons/core/translations) within Core Addon, check the specific [translations Readme](https://github.com/hashicorp/boundary-ui/blob/main/addons/core/translations/README.md) to learn more about it.

To learn more how to use a translation on the templates checkout [the `t` helper documentation](https://ember-intl.github.io/ember-intl/docs/helpers/t).
To learn more how to use a translation on the templates checkout [the `t` helper documentation](https://ember-intl.github.io/ember-intl/docs/helpers/t).

### A11y Auditing

We use `a11yAudit()` in our acceptance tests to verify that we have no a11y violations in our app, for both light and dark theme. Every route needs to be audited along with each change on the DOM. If a user can take an action that causes the UI to change within the scope of that route, we should audit that change. For example, we should audit the listing of auth methods, audit when we filter auth methods, and when any dropdowns or modals show up on the page.

Since the Admin UI and Desktop both have light and dark color themes we must audit both. To address this effectively, we use `test.each()` to loop over a11y test. Once for light theme and again for dark theme. See [auth-methods list](https://github.com/hashicorp/boundary-ui/blob/main/ui/admin/tests/acceptance/auth-methods/list-test.js) for how these tests should be structured.
94 changes: 87 additions & 7 deletions ui/admin/tests/acceptance/auth-methods/list-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@
*/

import { module, test } from 'qunit';
import { visit, click, waitFor, fillIn } from '@ember/test-helpers';
import { visit, click, waitFor, fillIn, currentURL } from '@ember/test-helpers';
import { setupApplicationTest } from 'ember-qunit';
import setupMirage from 'ember-cli-mirage/test-support/setup-mirage';
import { setupIndexedDb } from 'api/test-support/helpers/indexed-db';
import a11yAudit from 'ember-a11y-testing/test-support/audit';
import {
currentSession,
authenticateSession,
// These are left here intentionally for future reference.
//currentSession,
//invalidateSession,
} from 'ember-simple-auth/test-support';
import { Response } from 'miragejs';
import {
TYPE_AUTH_METHOD_PASSWORD,
TYPE_AUTH_METHOD_OIDC,
Expand Down Expand Up @@ -47,6 +47,8 @@ module('Acceptance | auth-methods | list', function (hooks) {
globalScope: null,
orgScope: null,
authMethods: null,
orgAuthMethods: null,
globalAuthMethods: null,
passwordAuthMethod: null,
oidcAuthMethod: null,
};
Expand All @@ -69,13 +71,75 @@ module('Acceptance | auth-methods | list', function (hooks) {
scope: instances.scopes.org,
type: TYPE_AUTH_METHOD_OIDC,
});
urls.globalScope = `/scopes/global/scopes`;
urls.globalScope = `/scopes/global`;
urls.orgScope = `/scopes/${instances.scopes.org.id}`;
urls.authMethods = `/scopes/${instances.scopes.org.id}/auth-methods`;
urls.globalAuthMethods = `${urls.globalScope}/auth-methods`;
urls.orgAuthMethods = `${urls.orgScope}/auth-methods`;
urls.passwordAuthMethod = `${urls.authMethods}/${instances.passwordAuthMethod.id}`;
urls.oidcAuthMethod = `${urls.authMethods}/${instances.oidcAuthMethod.id}`;

authenticateSession({});
authenticateSession({ username: 'admin' });
});

module('a11yAudit', function () {
test.each('auth-methods', ['light', 'dark'], async function (assert, data) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big fan of using test.each for this, I didn't realize this actually existed and wanted to parametrize some tests before and would've loved to have done this instead!

assert.expect(0);
currentSession().set('data.theme', data);
await visit(urls.authMethods);

// open new dropdown
await click('.rose-layout-page-actions button');
await a11yAudit();

// open dropdown at last row
await click('td:last-child button');
await a11yAudit();

// open primary filter
await click('.hds-segmented-group div[name="primary"] div button');
await a11yAudit();

// check 'yes'
await click(
'.hds-segmented-group div[name="primary"] div:last-child input',
);
await a11yAudit();

// filter selected
await click(FILTER_DROPDOWN_SELECTOR('type'));
await click(`input[value="${TYPE_AUTH_METHOD_PASSWORD}"]`);
await click(FILTER_APPLY_BUTTON_SELECTOR);
await a11yAudit();

// "no results" message
await fillIn(
SEARCH_INPUT_SELECTOR,
'fake auth method that does not exist',
);
await a11yAudit();
});

test('API error', async function (assert) {
assert.expect(0);
this.server.get('/auth-methods', () => {
return new Response(
418,
{},
{
status: 418,
code: "I'm a teapot",
message: 'Ope, sorry about that!',
},
);
});

await visit(urls.globalAuthMethods);
await a11yAudit();

currentSession().set('data.theme', 'dark');
await a11yAudit();
});
});

test('users can navigate to auth methods with proper authorization', async function (assert) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None blocking comment: the group of tests which do not belong to the a11Audit aren't group by a module statement. Is this in purpose?
Following the pattern we have in the acceptance test, i.e: users list-test, auth-methods delete-test, we wrap them in a module, such as 'Acceptance | auth-methods | list'.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out as looking at this is confusing. All the tests in this file are wrapped in a module named 'Acceptance | auth-methods | list'. What I did was add another module within that just for the a11yAudit tests. The main reason for this is we get access to all of the work done in the beforeEach. I wonder if maybe I should move the whole a11yAudit module to the bottom of the parent test module to make this less confusing?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh gotcha. Now I see it,
So the a11yAudit block is within the Acceptance | auth-methods | list block.

Yeah, not a strong opinion which can be the best formula for that. I am fine with it 😉 and you are leading this initiative, so whatever you think is best, let's keep with it and we can review later if its a problem.

Great work!!

Expand Down Expand Up @@ -116,6 +180,22 @@ module('Acceptance | auth-methods | list', function (hooks) {
assert.dom(`[href="${urls.authMethods}"]`).exists();
});

test('visiting auth methods in org scope', async function (assert) {
await visit(urls.orgScope);

await click(`[href="${urls.orgAuthMethods}"]`);

assert.strictEqual(currentURL(), urls.orgAuthMethods);
});

test('visiting auth methods in global scope', async function (assert) {
await visit(urls.globalScope);

await click(`[href="${urls.globalAuthMethods}"]`);

assert.strictEqual(currentURL(), urls.globalAuthMethods);
});

test('user can search for a specifc auth-method by id', async function (assert) {
await visit(urls.orgScope);

Expand All @@ -139,7 +219,7 @@ module('Acceptance | auth-methods | list', function (hooks) {
assert.dom(`[href="${urls.passwordAuthMethod}"]`).exists();
assert.dom(`[href="${urls.oidcAuthMethod}"]`).exists();

await fillIn(SEARCH_INPUT_SELECTOR, 'fake target that does not exist');
await fillIn(SEARCH_INPUT_SELECTOR, 'fake auth method that does not exist');
await waitFor(NO_RESULTS_MSG_SELECTOR, { count: 1 });

assert.dom(`[href="${urls.passwordAuthMethod}"]`).doesNotExist();
Expand Down
18 changes: 0 additions & 18 deletions ui/admin/tests/acceptance/auth-methods/read-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,24 +72,6 @@ module('Acceptance | auth-methods | read', function (hooks) {
urls.oidcAuthMethodGlobal = `${urls.globalAuthMethods}/${instances.oidcAuthMethodGlobal.id}`;
});

test('visiting auth methods in org scope', async function (assert) {
await visit(urls.orgScope);

await click(`[href="${urls.orgAuthMethods}"]`);
await a11yAudit();

assert.strictEqual(currentURL(), urls.orgAuthMethods);
});

test('visiting auth methods in global scope', async function (assert) {
await visit(urls.globalScope);

await click(`[href="${urls.globalAuthMethods}"]`);
await a11yAudit();

assert.strictEqual(currentURL(), urls.globalAuthMethods);
});

test('can navigate to an auth method form in org scope', async function (assert) {
await visit(urls.orgScope);

Expand Down