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

Jump to bottom

[Storage] Add sign out test #2122

Merged
merged 1 commit into from
May 10, 2022
Merged

[Storage] Add sign out test #2122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion packages/storage-ui/cypress/support/page-objects/navigationMenu.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ export const navigationMenu = {
spaceUsedLabel: () => cy.get("[data-cy=label-space-used]"),
spaceUsedProgressBar: () => cy.get("[data-cy=progress-bar-space-used]"),
// mobile view only
signOutButton: () => cy.get("[data-cy=container-signout-nav]")
signOutButton: () => cy.get("[data-cy=button-sign-out]")
}
29 changes: 24 additions & 5 deletions packages/storage-ui/cypress/tests/main-navigation-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,34 @@
import { navigationMenu } from "../support/page-objects/navigationMenu"
import { cidsPage } from "../support/page-objects/cidsPage"
import { authenticationPage } from "../support/page-objects/authenticationPage"

describe("Main Navigation", () => {

context("desktop", () => {
before(() => {
beforeEach(() => {
cy.web3Login()
})

it("can navigate to the cids page", () => {
navigationMenu.cidsNavButton().click()
cy.url().should("include", "/cids")
})
})

context("mobile", () => {
before(() => {
cy.web3Login()
it("can sign out from the navigation bar", () => {
navigationMenu.signOutDropdown().click()
navigationMenu.signOutMenuOption()
.should("be.visible")
.click()
authenticationPage.web3Button().should("be.visible")
cy.url().should("not.include", "/drive")
cy.url().should("not.include", "/bin")
cy.url().should("not.include", "/settings")
})
})

context("mobile", () => {
beforeEach(() => {
cy.web3Login()
cy.viewport("iphone-6")
cidsPage.hamburgerMenuButton().click()
})
Expand All @@ -28,5 +37,15 @@ describe("Main Navigation", () => {
navigationMenu.cidsNavButton().click()
cy.url().should("include", "/cids")
})

it("can sign out from the navigation bar", () => {
navigationMenu.signOutButton()
.should("be.visible")
.click()
authenticationPage.web3Button().should("be.visible")
cy.url().should("not.include", "/drive")
cy.url().should("not.include", "/bin")
cy.url().should("not.include", "/settings")
})
})
})
1 change: 1 addition & 0 deletions packages/storage-ui/src/Components/Layouts/AppNav.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@ const AppNav: React.FC<IAppNav> = ({ navOpen, setNavOpen }: IAppNav) => {

<div style={{ display: "flex" }}>
<Button
data-cy="button-sign-out"
Copy link
Member

Choose a reason for hiding this comment

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

The button component has a testId prop so instead of the custom tag data-cy tag you could also write

testId="sign-out" and your locator would be [data-testid=button-sign-out]

It's a minor thing but I think when the component supports testId props it's probably better to use them as it's simpler, cleaner and makes sense to people reading the code for the first time. What do you think?

PS there may be other examples in the framework where we aren't using the available props as in some cases the component was updated after, we can clean those up as we encounter them.

Copy link
Collaborator

Choose a reason for hiding this comment

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

My 2cts here, testId props should only be available for things where the data-cy doesn't work. I think we should use the data-cy as much as possible. To me, this is a mistake to have a testId prop if the data-cy works. This is definitely something we should look into.

Copy link
Collaborator

Choose a reason for hiding this comment

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

what testId does is that it passes down a data-testId identifier to the underlying html component, if the html component isn't accessible from the code from Files. This is the case for some component from our library, but there are few afaik.

Copy link
Member

Choose a reason for hiding this comment

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

Ok! Let's exclusively only use testId when regular data-cy doesn't work then, sorry for any confusion @juans-chainsafe

variant='secondary'
onClick={() => {
handleOnClick()
Expand Down