-
Notifications
You must be signed in to change notification settings - Fork 4.5k
[UI] [VAULT-36185] Acceptance tests for Reporting #30627
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
Conversation
|
Build Results: |
|
CI Results: |
| test('it renders the counters dashboard block with all expected counters', async function (assert) { | ||
| this.server.get('http://localhost:7357/v1/sys/utilization-report', () => mockedResponseWithData); | ||
| await visit('/vault/usage-reporting'); | ||
| await waitFor('[data-test-dashboard-counters]'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Up to you guys, but I recommend creating a vault-usage helper directory & file. Similar to something like this.
| await waitFor('[data-test-dashboard-secret-engines]'); | ||
|
|
||
| const card = document.querySelector('[data-test-dashboard-secret-engines]'); | ||
| assert.ok(card, 'renders Secret engines card'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We try not to use assert.ok, but instead assert.true or assert.false
| this.server.get('http://localhost:7357/v1/sys/utilization-report', () => mockedResponseWithData); | ||
| await visit('/vault/usage-reporting'); | ||
| await waitFor('[data-test-dashboard-counters]'); | ||
| assert.dom('[data-test-dashboard-counters]').exists('renders the counters dashboard block'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To help avoid confusion with other areas of our app that have the concept of dashboard, could we change these selectors here to something like data-test-usage-counters? If you do a quick search of the app you can see the use of data-test-dashboard-... in several places.
| .includesText('Global lease count quota', 'Lease quota empty state: docs link is shown'); | ||
| }); | ||
|
|
||
| // |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // |
|
|
||
| const mockedResponseWithData = { | ||
| data: { | ||
| auth_methods: { cats: 42 }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the keys here auth method types or paths? I'm guessing paths because of the numerical number. For posterity it might be helpful to name these more similar to the real-world.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, they're paths. i updated their names and added more options.
9d12a42 to
e640f22
Compare
9102ae8 to
40e75f6
Compare
40e75f6 to
1544ba1
Compare
| }); | ||
|
|
||
| test('it renders the counters dashboard block with all expected counters', async function (assert) { | ||
| this.server.get('http://localhost:7357/v1/sys/utilization-report', () => mockedResponseWithData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typically we remove the origin from the url example when stubbing mirage responses so that if something with the host in CI changes tests won't fail. Removing v1 should also be possible because the server should automatically prefix that
| this.server.get('http://localhost:7357/v1/sys/utilization-report', () => mockedResponseWithData); | |
| this.server.get('sys/utilization-report', () => mockedResponseWithData); |
Co-authored-by: Angel Garbarino <Monkeychip@users.noreply.github.com>
| await login(); | ||
| }); | ||
|
|
||
| test('it visits the usage reporting dashboard and renders the header', async function (assert) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this test is asserting navigation, I think it would be a little more resilient to click the sidebar nav link rather than visit the route directly. That way we get the added benefit of testing the conditionals wrapping the link (which from what I could tell, currently only have integration tests)
| await visit('/vault/usage-reporting'); | ||
| await waitFor('[data-test-vault-reporting-dashboard-secret-engines]'); | ||
|
|
||
| assert.dom('[data-test-vault-reporting-dashboard-secret-engines]').exists('renders Secret engines card'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not necessary to update, but mostly an FYI. We typically use const SELECTORS var at either the top of the test file, or in a separate helper file if they'll be reused elsewhere, to streamline future test selector updates. Here's an example of a dynamic selector so that the test file just references SELECTORS.card('secret-engines').
|
|
||
| test('it visits the usage reporting dashboard and renders the header', async function (assert) { | ||
| await visit('/vault/dashboard'); | ||
| await click('[data-test-sidebar-nav-link="Vault Usage"]'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏 awesome, thanks!
Description
[UI] VAULT-36185 Acceptance tests for Reporting
Adds acceptance tests for reporting dashboard.
TODO only if you're a HashiCorp employee
backport/label that matches the desired release branch. Note that in the CE repo, the latest release branch will look likebackport/x.x.x, but older release branches will bebackport/ent/x.x.x+ent.of a public function, even if that change is in a CE file, double check that
applying the patch for this PR to the ENT repo and running tests doesn't
break any tests. Sometimes ENT only tests rely on public functions in CE
files.
in the PR description, commit message, or branch name.
description. Also, make sure the changelog is in this PR, not in your ENT PR.