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

Add a cypress test for testing project basic operations: create, edit, delete #3881

Merged

Conversation

lorenzo-cavazzi
Copy link
Member

See title

/deploy renku-ui=lorenzo/cypress-renku-2.0-data-cy

fix #3880

@RenkuBot
Copy link
Collaborator

You can access the deployment of this PR at https://ci-renku-3881.dev.renku.ch

@lorenzo-cavazzi lorenzo-cavazzi marked this pull request as ready for review January 29, 2025 12:15
@lorenzo-cavazzi lorenzo-cavazzi requested a review from a team as a code owner January 29, 2025 12:15
@leafty leafty self-assigned this Jan 29, 2025
cy.getDataCy("project-visibility-public").click();
cy.getDataCy("project-description-input").type(projectDescription);
cy.getDataCy("new-project-modal").contains(
`The URL for this project will be renkulab.io/v2/projects/${username}/${projectPath}`
Copy link
Member

Choose a reason for hiding this comment

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

Oh my, oh my, oh my, this is actually hard-coded 🙈 Ouch! 🤕

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Please remove or comment this test.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am checking to ensure we are showing the correct expected URL.
What would be a reasonable alternative here? Perhaps just /${username}/${projectPath}?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe test that for now and have a comment. Also, that URL WILL be different at the end of this build cycle.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, true. I made this more resilient to the URL changes and, following your other suggestion on dropping the matcher for new-project-modal, I now match the URL element more precisely 2061f52

Copy link
Member

@leafty leafty left a comment

Choose a reason for hiding this comment

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

Thanks for adding this! 👏

Some comments below.

cypress-tests/cypress/e2e/v2/projectBasics.cy.ts Outdated Show resolved Hide resolved
cypress-tests/cypress/e2e/v2/projectBasics.cy.ts Outdated Show resolved Hide resolved
cypress-tests/cypress/e2e/v2/projectBasics.cy.ts Outdated Show resolved Hide resolved
cypress-tests/cypress/e2e/v2/projectBasics.cy.ts Outdated Show resolved Hide resolved
cypress-tests/cypress/e2e/v2/projectBasics.cy.ts Outdated Show resolved Hide resolved
cypress-tests/cypress/support/commands/general.ts Outdated Show resolved Hide resolved
cypress-tests/cypress/support/commands/general.ts Outdated Show resolved Hide resolved
@lorenzo-cavazzi
Copy link
Member Author

@leafty thank you for the review!
I implemented all the suggestions except the ones asking to check for the element text after getting it though getDataCy.
The reason is here #3881 (comment) . I'm happy to further discuss it offline if you disagree or prefer a different approach.

const username = user.username;
cy.getDataCy("navbar-new-entity").click();
cy.getDataCy("navbar-project-new").click();
cy.getDataCy("new-project-modal").should("exist");
Copy link
Member

Choose a reason for hiding this comment

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

From the data-cy conversation, I think we should instead check against the presence of a new project form and not a modal. The fact that the form is presented inside a modal is not important and could change (a page, an off canvas, etc.) but there should always be a data-cy=new-project-form element presented to the user.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this makes sense! I dropped the use of new-project-modal and match the form instead 2061f52

Copy link
Member

@leafty leafty left a comment

Choose a reason for hiding this comment

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

Excellent 👌

@lorenzo-cavazzi lorenzo-cavazzi force-pushed the lorenzo/cypress-renku-2.0 branch from aa689e2 to cbe29dc Compare January 31, 2025 13:31
@lorenzo-cavazzi lorenzo-cavazzi force-pushed the lorenzo/cypress-renku-2.0-project branch from 2061f52 to ac463a6 Compare January 31, 2025 13:33
@lorenzo-cavazzi lorenzo-cavazzi merged commit b0a62ab into lorenzo/cypress-renku-2.0 Jan 31, 2025
18 checks passed
@lorenzo-cavazzi lorenzo-cavazzi deleted the lorenzo/cypress-renku-2.0-project branch January 31, 2025 14:23
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