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.

Sign up for GitHub

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

Remaining watch account ui tests #446

Merged
merged 10 commits into from
Dec 1, 2023
Merged

Remaining watch account ui tests #446

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
784e664
add new element locators
asnaith Nov 29, 2023
318a655
update test fixture
asnaith Nov 30, 2023
ed4be59
add new test scenarios
asnaith Nov 30, 2023
983b679
improve test
asnaith Nov 30, 2023
0a65852
tidy
asnaith Nov 30, 2023
104e08a
Merge branch 'main' into asnaith/last-watched-account-tests
asnaith Nov 30, 2023
912d8a9
Remove it.only
asnaith Nov 30, 2023
e7f1a52
Update packages/ui/cypress/fixtures/watchAccounts/watchSignatories.ts
asnaith Nov 30, 2023
312cfca
address pr feedback
asnaith Nov 30, 2023
b49a6d7
Use a function in tests to verify the same info are displayed (#450)
Tbaut Dec 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions packages/ui/cypress/fixtures/watchAccounts/watchSignatories.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,20 @@ export const watchSignatories = [
mnemonic: 'mutual pluck punch boy gym key brush dune master aunt track dynamic'
}
]

// signatory that is a member of 3 multisigs. 2 multisig with a pure, 1 multisig without a pure
export const signatoryOfMultipleMultisigs = {
address: '5CUZNrH9eMCvyz3wL5n8V9oJuaaNPKWCGCZZj7UfUV4otHMN',
name: 'Signatory of Multiple Multisigs',
type: 'sr25519',
mnemonic: 'cement risk mutual large fluid run witness vintage civil pipe cost weasel',
multisigWithPure1: {
address: '5EKfTP24LJvTDLf3GP3vvJASv1jXmcZumFWGcSzVQ66epXZe'
},
multisigWithPure2: {
address: '5ENJzVn5CcVrzSi9dfmysrFWQ6Ce5iX3L22sa1bnZN8KvGBT'
},
multisigWithoutPure: {
address: '5HoPXnwfDpqhgNYGMSZTzhdvc3TDSvd41XDaVMg7yUo8W5un'
}
asnaith marked this conversation as resolved.
Show resolved Hide resolved
}
11 changes: 10 additions & 1 deletion packages/ui/cypress/support/page-objects/multisigPage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export const multisigPage = {
// header elements
accountHeader: (timeout = 4000) => cy.get('[data-cy=header-account]', { timeout }),
seeOverviewButton: () => cy.get('[data-cy=button-see-overview]'),
newTransactionButton: () => cy.get('[data-cy=button-new-transaction]'),
Expand All @@ -8,5 +9,13 @@ export const multisigPage = {
transactionList: () => cy.get('[data-cy=container-transaction-list]', { timeout: 20000 }),
pendingTransactionItem: () => cy.get('[data-cy=container-pending-tx-item]'),
reviewButton: () => cy.get('[data-cy=button-review-tx]'),
setIdentityMenuOption: () => cy.get('[data-cy=menu-option-set-identity]')
setIdentityMenuOption: () => cy.get('[data-cy=menu-option-set-identity]'),

// multisig details elements
multisigDetailsContainer: () => cy.get('[data-cy=container-multisig-details]'),
multisigAccountSummary: () => cy.get('[data-cy=container-multisig-account-summary]'),
thresholdListItem: () => cy.get('[data-cy=list-item-threshold]'),
balanceListItem: () => cy.get('[data-cy=list-item-balance]'),
signatoriesAccordion: () => cy.get('[data-cy=accordion-signatories]'),
expandSignatoriesIcon: () => cy.get('[data-cy=icon-expand-signatories-summary]')
asnaith marked this conversation as resolved.
Show resolved Hide resolved
}
101 changes: 100 additions & 1 deletion packages/ui/cypress/tests/watched-accounts.cy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { watchMultisigs } from '../fixtures/watchAccounts/watchMultisigs'
import { multisigPage } from '../support/page-objects/multisigPage'
import { editNamesModal } from '../support/page-objects/modals/editNamesModal'
import { testAccounts } from '../fixtures/testAccounts'
import { signatoryOfMultipleMultisigs } from '../fixtures/watchAccounts/watchSignatories'

const addWatchAccount = (address: string, name?: string) => {
settingsPage.accountAddressInput().type(`${address}{enter}`, { delay: 20 })
Expand Down Expand Up @@ -82,7 +83,6 @@ describe('Watched Accounts', () => {
},
watchedAccounts: [multisigPublicKey]
})

// ensure the multisig name is displayed in the settings account container
settingsPage.accountContainer().within(() => {
accountDisplay.identicon().should('be.visible')
Expand Down Expand Up @@ -239,4 +239,103 @@ describe('Watched Accounts', () => {
multisigPage.reviewButton().should('not.exist')
})
})

it('can see the same details displayed when watching a pure or its multisig address', () => {
const { purePublicKey, address: multisigAddress } = watchMultisigs['multisig-with-pure']

cy.visitWithLocalStorage({
url: landingPageUrl,
watchedAccounts: [purePublicKey]
})
// store details of the displayed addresses for later comparison
multisigPage.accountHeader().within(() => {
accountDisplay.addressLabel().should('be.visible').invoke('text').as('headerPureAddress')
Copy link
Collaborator

Choose a reason for hiding this comment

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

I've been searching for a way to achieve the same, in an easier to read way. I've always found aliases pretty convoluted, with the @ etc.

This is nice and generic, but can't we just compare it to what we expect? After all this is all just testing that the pure (and down the file the multisig) address is the same isn't it? It would make this test easier to read and maintain IMHO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What I preferred about using the aliases is that when we were inspecting the UI and watching via the pure we were dynamically grabbing and storing whatever data was displayed (without caring, just knowing this is the view when a pure is watched) and then only asserting when we watch via the multisig that the displayed details are the same as what we saw previously.

I've updated now to compare using the static values as requested but this means we are now first asserting on the display when watching a pure key (already covered in other tests) and then again the displayed data when watching a multisig key, a trivial difference that probably makes no change in execution time but it's less of a well-written test in my opinion. We break test writing conventions of only asserting after the point of change (but admittedly we do this all the time especially with should('be.visible') checks because we have to 😄).

Ultimately if it reads better I guess that matters more but I wanted to explain my view. I still think cypress alias will have their place when checking dynamic data but perhaps it makes less sense when values are static like this (and I do agree the @ syntax is a little odd!).

Copy link
Collaborator

@Tbaut Tbaut Dec 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe with my PR using the exact same code, this reads even better, this is how I was thinking about this test originally. Sure it's a bit dumb and we're checking twice the same things (now not repeated, but in a function) but how we arrived at this state is different, and that's what matters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep this reads well, thanks :)

})
multisigPage.multisigDetailsContainer().within(() => {
multisigPage.multisigAccountSummary().within(() => {
accountDisplay
.addressLabel()
.should('be.visible')
.invoke('text')
.as('controllingMultisigAddress')
})
})
topMenuItems.multiproxySelector().should('be.visible').first().click()
asnaith marked this conversation as resolved.
Show resolved Hide resolved
topMenuItems
.multiproxySelectorOption()
// ensure there is only one option, the pure, in the multiproxy selector
.should('have.length', 1)
.within(() => {
accountDisplay.pureBadge().should('be.visible')
accountDisplay.addressLabel().should('be.visible').invoke('text').as('dropdownPureAddress')
asnaith marked this conversation as resolved.
Show resolved Hide resolved
})
// now watch via the multisig address and ensure that we see the same details displayed
cy.visitWithLocalStorage({
url: landingPageUrl,
watchedAccounts: [multisigAddress]
Tbaut marked this conversation as resolved.
Show resolved Hide resolved
})
multisigPage.accountHeader().within(() => {
cy.get<string>('@headerPureAddress').then((headerPureAddress) => {
accountDisplay.addressLabel().should('have.text', headerPureAddress)
})
})
multisigPage.multisigDetailsContainer().within(() => {
multisigPage.multisigAccountSummary().within(() => {
cy.get<string>('@controllingMultisigAddress').then((controllingMultisigAddress) => {
accountDisplay.addressLabel().should('have.text', controllingMultisigAddress)
})
})
})
topMenuItems.multiproxySelector().should('be.visible').first().click()
topMenuItems
.multiproxySelectorOption()
// ensure there is still only one option, the pure, in the multiproxy selector
.should('have.length', 1)
.within(() => {
accountDisplay.pureBadge().should('be.visible')
cy.get<string>('@dropdownPureAddress').then((dropdownPureAddress) => {
accountDisplay.addressLabel().should('have.text', dropdownPureAddress)
})
})
})

it('can see all multisigs that a watched signatory is a member of', () => {
const signatory = signatoryOfMultipleMultisigs.address
const expectedAddresses = [
signatoryOfMultipleMultisigs.multisigWithPure1.address,
signatoryOfMultipleMultisigs.multisigWithPure2.address,
signatoryOfMultipleMultisigs.multisigWithoutPure.address
]

cy.visitWithLocalStorage({
url: landingPageUrl,
watchedAccounts: [signatory]
Tbaut marked this conversation as resolved.
Show resolved Hide resolved
})
topMenuItems.multiproxySelector().should('be.visible').first().click()
// ensure all multisigs are displayed in the multiproxy selector
topMenuItems
.multiproxySelectorOption()
.should('have.length', 3)
.each(($el, index) => {
cy.wrap($el).within(() => {
accountDisplay.addressLabel().should('contain.text', expectedAddresses[index].slice(0, 6))
if (index < 2) {
accountDisplay.pureBadge().should('exist')
} else {
accountDisplay.multisigBadge().should('exist')
}
})
})
// ensure each multisig that the signatory is a member of can be viewed
expectedAddresses.forEach((address, index) => {
topMenuItems.multiproxySelector().first().click()
topMenuItems.multiproxySelectorOption().eq(index).click()
multisigPage
.accountHeader()
.should('be.visible')
.within(() => {
accountDisplay.addressLabel().should('contain.text', address.slice(0, 6))
})
})
})
})
8 changes: 6 additions & 2 deletions packages/ui/src/pages/Home/MultisigAccordion.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,17 @@ interface AccordionProps {

const MultisigAccordion = ({ multisig }: AccordionProps) => {
return (
<AccordionMuiStyled TransitionProps={{ timeout: { appear: 300, enter: 300, exit: 0 } }}>
<AccordionMuiStyled
TransitionProps={{ timeout: { appear: 300, enter: 300, exit: 0 } }}
data-cy="accordion-signatories"
>
<AccordionSummary
expandIcon={
<HiOutlineChevronDown
color="black"
size="1rem"
strokeWidth="2"
data-cy="icon-expand-signatories-summary"
/>
}
>
Expand All @@ -27,7 +31,7 @@ const MultisigAccordion = ({ multisig }: AccordionProps) => {
</AccordionSummary>
<AccordionDetails>
<MultisigPaperStyled key={multisig.address}>
<AddressListStyled>
<AddressListStyled data-cy="list-item-signatory">
{multisig?.signatories?.map((signatory) => (
<li key={signatory}>
<AccountDisplay address={signatory} />
Expand Down
7 changes: 4 additions & 3 deletions packages/ui/src/pages/Home/MultisigView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@ const MultisigView = () => {
<MultisigWrapperStyled
selectedHasProxy={selectedHasProxy}
key={multisig.address}
data-cy="container-multisig-details"
>
{selectedHasProxy && (
<AccountDisplayWrapperStyled>
<AccountDisplayWrapperStyled data-cy="container-multisig-account-summary">
<AccountDisplay
address={multisig.address || ''}
badge={AccountBadge.MULTI}
Expand All @@ -41,12 +42,12 @@ const MultisigView = () => {
</AccountDisplayWrapperStyled>
)}
<List>
<ListElement>
<ListElement data-cy="list-item-threshold">
<ListFieldText>Threshold</ListFieldText>
<ChipStyled label={`${multisig.threshold}/${multisig.signatories?.length}`} />
</ListElement>
{selectedHasProxy && (
<ListElement>
<ListElement data-cy="list-item-balance">
<ListFieldText>Balance</ListFieldText>
<Balance address={multisig.address} />
</ListElement>
Expand Down
Loading