Skip to content

chore(ui5-popover): migrate wdio tests to cypress #11757

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

duygu-rmdn
Copy link
Contributor

Copy link
Contributor

@nnaydenow nnaydenow left a comment

Choose a reason for hiding this comment

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

dont select by id where it's possible. use [custom-tag] selector instead

cy.mount(
<>
<Button id="popoverOpen">Open</Button>
<Popover id="popover" opener="popoverOpen" open={true}>
Copy link
Contributor

Choose a reason for hiding this comment

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

this test doesn't match the description

cy.mount(
<>
<Button id="popoverOpen">Open</Button>
<Popover id="popover1" opener="popoverOpen" open={true}>
Copy link
Contributor

Choose a reason for hiding this comment

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

split in 2 test if applicable

</>
);

// act
Copy link
Contributor

Choose a reason for hiding this comment

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

you already tested it with providing inital text. invoke is redudnatn

cy.get("#btn").realClick();

// Assert: Initially, the popover has an arrow
cy.get("#pop")
Copy link
Contributor

Choose a reason for hiding this comment

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

split this into 2 tests - one for checking the arrow and one when it's hidden

cy.get("[ui5-popover]")
.shadow()
.find(".ui5-popup-root")
.should("have.attr", "aria-labelledby");
Copy link
Contributor

Choose a reason for hiding this comment

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

split this into 2 tests: one for default text and one for custom

<Button id="btnPopModal">Open Modal Popover</Button>
<Popover id="modalPopover" opener="btnPopModal" modal open={false}>
<Button id="modalPopoverClose" onClick={() => {
const popover = document.getElementById("modalPopover");
Copy link
Contributor

Choose a reason for hiding this comment

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

this event listener is redudant. it's the same as to call invoke(prop, open, false) to the popover.

);

// Act: Open the modal popover
cy.get("#btnPopModal").click();
Copy link
Contributor

Choose a reason for hiding this comment

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

realClick instead of click

cy.get("#modalPopoverNoLayer").should("have.attr", "open", "open");

// Act: Close the popover using Escape key
cy.get("body").realPress("Escape");
Copy link
Contributor

Choose a reason for hiding this comment

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

realPress doesn't need to be called on exact element

cy.get("body").realPress("Escape");

// Assert: Popover is closed
cy.get("#modalPopoverNoLayer").should("not.be.visible");
Copy link
Contributor

Choose a reason for hiding this comment

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

probably check that the block layer is not visible when the popover is opened?

cy.get("body").then(body => {
let container = document.createElement("div");
container.id = "container";
body.get(0).appendChild(container);
Copy link
Contributor

Choose a reason for hiding this comment

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

append element to [data-cy-root] element otherwise the element will remain for next tests

@nnaydenow
Copy link
Contributor

some comments apply for the whole file so please check when addressing them.

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.

3 participants