-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
UI: dashboard acceptance tests for quick actions, client count, replication #22475
UI: dashboard acceptance tests for quick actions, client count, replication #22475
Conversation
Build Results: |
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.
Looks good! Just some suggestions about renaming some selectors to make them more reusable - but nothing blocking! 🚀
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.
Do all of the getters and things update as expected? I know sometimes ember has trouble tracking nested values if tracked properties are objects. It looks like tests would account for that, but just wanted to check 😄
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.
@hellobontempo I was concerned about that too! Do you think I should move it back to individual tracked properties? From my manual qa and my acceptance tests, everything works as intended, but I'm paranoid
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.
If I remember correctly - all of your getters recompute because of the selected engine 🤔
I think as long as you're not expecting the view to change based just the key value of the object, then we should be good!
<EmptyState | ||
@title="No mount selected" | ||
@message="Select a mount above to get started." | ||
data-test-no-mount-selected-empty |
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.
It seems like you could use the empty state component test selectors and just reuse those in your test file?
@@ -2,7 +2,7 @@ | |||
Copyright (c) HashiCorp, Inc. | |||
SPDX-License-Identifier: BUSL-1.1 | |||
~}} | |||
<Hds::Card::Container @hasBorder={{true}} class="has-padding-l has-bottom-padding-m"> | |||
<Hds::Card::Container @hasBorder={{true}} class="has-padding-l has-bottom-padding-m" data-test-replication-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.
if this selector is something like data-test-card="replication"
the same selector could be reused for your other cards in tests
SELECTORS = {
...
card: (name) => `[data-test-card="${name}"]`
}
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.
Thanks for pointing this out and great idea! I will make a follow up ticket!
assert.dom('[data-test-secrets-engines-select]').exists({ count: 1 }); | ||
assert.dom('[data-test-component="empty-state"]').exists({ count: 1 }); | ||
assert.dom(SELECTORS.secretsEnginesSelect).exists({ count: 1 }); | ||
assert.dom(SELECTORS.emptyState).exists({ count: 1 }); |
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.
I think asserting the empty state message and/or title have the correct text is a more honest check that the UI renders as expected
Description
Other things added:
kv version 1/2