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

SLVUU-49: Improve Cypress Selectors #128

Closed
wants to merge 14 commits into from

Conversation

pling-scottlogic
Copy link

Description

There is currently one Cypress test for the layout persistence epic, which uses brittle selectors such as CSS class names. The changes here improve the existing selectors to be more accessible and represent user interaction more accurately. We also establish a more robust testing pattern by way of the Page Object Model (POM), in which more accessible selectors are included to allow for future end-to-end testing of the layout management user journey.

Change List

  • Introduce Page Object Model (POM), abstracting accessible selectors for:
    • "ShellWithNewTheme" showcase example
    • Save Layout Dialog
  • Add accessible properties to the following components to improve selectors:
    • Stack (aria-label)
    • LayoutList (role)
    • Tabstrip (role)
  • Refactor existing Cypress test to use POM
  • Rename exported LayoutList component to match filename
  • Add "task" to Cypress config to allow console logging from tests

Testing

Temporarily inserted all new POM methods into screenshot test to ensure selectors are working as expected.

@pling-scottlogic pling-scottlogic self-assigned this Dec 4, 2023
@pling-scottlogic pling-scottlogic linked an issue Dec 4, 2023 that may be closed by this pull request

context("Screenshot", () => {
beforeEach(() => {
cy.visit(SHELL_WITH_NEW_THEME_URL);
page.visit();
});

// TODO (#VUU24): Improve test alignment with the user flow
Copy link
Author

Choose a reason for hiding this comment

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

Can we remove this comment now?

Copy link

@cfisher-scottlogic cfisher-scottlogic Dec 5, 2023

Choose a reason for hiding this comment

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

Yeah we've got plenty of tickets which should cover this now.

Comment on lines 24 to 30
getSavedLayoutButton(layoutName, creator, date) {
const day = ("00" + date.getDate()).slice(-2);
const formattedDate = `${day}.${date.getMonth() + 1}.${date.getFullYear()}`;
const elementName = `${layoutName} ${creator}, ${formattedDate}`;

return cy.findByRole("button", { name: elementName });
}
Copy link
Author

Choose a reason for hiding this comment

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

I feel like there must be a better way to do this. Suggestions welcome.

Choose a reason for hiding this comment

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

I think here we can use formatDate from vuu-utils like such: const formattedDate = formatDate(date, "dd.mm.yyyy")

Choose a reason for hiding this comment

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

In terms of a user point of view, I would imagine the ideal flow would be something more along the lines of what you have in getContextMenuButton. So get all layouts in my layouts -> find group x, find the one with y name or z date.

Copy link
Author

Choose a reason for hiding this comment

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

@vferraro-scottlogic Thanks, that's exactly what I was looking for!

@cfisher-scottlogic I think that would involve a bit of refactor in the LayoutList component, but it should be doable. I'll look into it.

Copy link
Author

Choose a reason for hiding this comment

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

It did require some refactor, but I think it was worth it: dc1810c

Choose a reason for hiding this comment

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

Nice, I think so too 🙂

Copy link

@vferraro-scottlogic vferraro-scottlogic left a comment

Choose a reason for hiding this comment

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

Change looks mostly good. I would like to see these new files to use typescript as the rest of the code in cypress/

Copy link

@cfisher-scottlogic cfisher-scottlogic left a comment

Choose a reason for hiding this comment

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

BEAUTIFUL! This is so much nicer.

vuu-ui/cypress/pages/ShellWithNewTheme.js Outdated Show resolved Hide resolved
Comment on lines 24 to 30
getSavedLayoutButton(layoutName, creator, date) {
const day = ("00" + date.getDate()).slice(-2);
const formattedDate = `${day}.${date.getMonth() + 1}.${date.getFullYear()}`;
const elementName = `${layoutName} ${creator}, ${formattedDate}`;

return cy.findByRole("button", { name: elementName });
}

Choose a reason for hiding this comment

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

In terms of a user point of view, I would imagine the ideal flow would be something more along the lines of what you have in getContextMenuButton. So get all layouts in my layouts -> find group x, find the one with y name or z date.

vuu-ui/cypress/pages/ShellWithNewTheme.js Outdated Show resolved Hide resolved
Copy link

@cfisher-scottlogic cfisher-scottlogic left a comment

Choose a reason for hiding this comment

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

I definitely didn't think you were just making these 'roles' up on the fly, and I definitely knew that they're predefined, and I definitely knew about the docs. I'm seeing this PR in a whole new light now 😅

Comment on lines 24 to 30
getSavedLayoutButton(layoutName, creator, date) {
const day = ("00" + date.getDate()).slice(-2);
const formattedDate = `${day}.${date.getMonth() + 1}.${date.getFullYear()}`;
const elementName = `${layoutName} ${creator}, ${formattedDate}`;

return cy.findByRole("button", { name: elementName });
}

Choose a reason for hiding this comment

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

Nice, I think so too 🙂

vuu-ui/cypress/pages/ShellWithNewTheme.js Outdated Show resolved Hide resolved
Copy link

@vferraro-scottlogic vferraro-scottlogic left a comment

Choose a reason for hiding this comment

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

Only a couple of questions and some recommendations around leveraging typescript in cypress code

vuu-ui/cypress/pages/SaveLayoutDialog.ts Outdated Show resolved Hide resolved
vuu-ui/cypress/pages/SaveLayoutDialog.ts Show resolved Hide resolved
vuu-ui/cypress/pages/ShellWithNewTheme.ts Show resolved Hide resolved
Copy link

@vferraro-scottlogic vferraro-scottlogic left a comment

Choose a reason for hiding this comment

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

Looks good :)

@pling-scottlogic
Copy link
Author

@pling-scottlogic
Copy link
Author

PR raised into Finos repo

Finos PR merged into main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cypress: add selectors
3 participants