Skip to content

feat(specs): adds cypress workflows#46

Merged
alokedesai merged 7 commits intowarpdotdev:mainfrom
jordanpowell88:add-cypress-workflow
Apr 25, 2022
Merged

feat(specs): adds cypress workflows#46
alokedesai merged 7 commits intowarpdotdev:mainfrom
jordanpowell88:add-cypress-workflow

Conversation

@jordanpowell88
Copy link
Copy Markdown
Contributor

This PR adds 4 cypress workflows

@elviskahoro elviskahoro requested a review from alokedesai April 17, 2022 07:02
@elviskahoro
Copy link
Copy Markdown

Thanks @jordanpowell88!

Copy link
Copy Markdown
Member

@alokedesai alokedesai left a comment

Choose a reason for hiding this comment

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

I have a small comment about clarifying what "mobile" means here. LGTM besides that though! Thank you so much for contributing!

Comment thread specs/cypress/run-cypress-mobile.yaml Outdated
@@ -0,0 +1,15 @@
name: Run Cypress Headless on Mobile
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm by no means an expert on cypress, but my understanding is this command is just setting the height and width to a certain size, regardless of whether it's a mobile device. It may be useful to name this workflow "Run Cypress Headless on Mobile Viewport" and the other one "Open Cypress in Mobile Viewport" to make that more clear

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback @alokedesai . Let me provide a bit more feedback in regards to Cypress. Essentially there are 2 different ways to "run" Cypress.

open essentially opens the cypress app and allows users to click on which tests they want to run, etc
run essentially runs the cypress app headless and tests every spec file.

It is a bit confusing but that is why we chose that specific verbiage in regards to open vs run.

Does this help?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for the context, the difference between "run" and "open" makes sense to me! Could we change "mobile" to "mobile viewport" though? I think that will help explain the actual command that's executed, which just hardcodes a small "mobile-sized" viewport

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

982f6ee makes this change

Copy link
Copy Markdown
Member

@alokedesai alokedesai left a comment

Choose a reason for hiding this comment

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

Thank you so much for contributing!

Comment thread specs/cypress/open-cypress-mobile-viewport.yaml Outdated
@elviskahoro
Copy link
Copy Markdown

@jordanpowell88 Are you in our Discord btw? Can you let me know your username so I can add the contributor role?

https://discord.gg/warpdotdev

@jordanpowell88
Copy link
Copy Markdown
Contributor Author

@jordanpowell88 Are you in our Discord btw? Can you let me know your username so I can add the contributor role?

https://discord.gg/warpdotdev

Yes. jordanpowell88

@alokedesai alokedesai merged commit 0050f61 into warpdotdev:main Apr 25, 2022
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