-
Notifications
You must be signed in to change notification settings - Fork 329
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
Cypress UI testing & verify template tests #153
base: ui-tests
Are you sure you want to change the base?
Cypress UI testing & verify template tests #153
Conversation
cypress/fixtures/users.json
Outdated
@@ -0,0 +1,232 @@ | |||
[ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these fixture files being used in the tests? They seem like generic files part of some auto configuration process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are example fixtures I left in as a placeholder for when we have mocks for integration tests, but it is a good idea to remove them. I do want to keep the directory around if possible, since it does have special meaning to Cypress (the data for API mocks go in .json
files in the fixtures directory). Should I keep a .gitignore
in this directory to keep it in git, or perhaps a README.md
or Fixtures.md
explaining how Cypress uses this directory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think having a README.md or a .gitkeep
file would make sense. I'd definitely get rid of the bigger fixtures
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the example files, and wrote up a quick README.md that links to the Cypress documentation on how the files that were in the fixtures directory should be used.
|
||
// `verify` function template UI-only tests | ||
|
||
const testNumber = "5555555555"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its a good habit to get used to formatting phone numbers with e.164 formatting. Here's probably more info than you need on the subject https://support.twilio.com/hc/en-us/articles/223183008-Formatting-International-Phone-Numbers
TLDR: Make sure to add the country code to the number "+15555555555"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd go one further and use specifically a valid phone number (since the one you listed is not valid). The typical one we have in examples is: +18448144627
I'm using it in this test for example: https://github.com/twilio-labs/configure-env/blob/main/src/__tests__/validators.test.ts#L22
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I switched it to Dom's suggestion of +18448144627
! I did notice that the interface has a dropdown for the country code with +1
filled in, and the testNumber
string only controls what's typed into the textbox next to the country code dropdown, so it's possible we may have to revisit this slightly if our future integration tests receive +1+18448144627
from the UI.
If you want to run Cypress tests for all of the function templates in your terminal, use: | ||
|
||
```bash | ||
npm cypress |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a command for running individual tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent question! I took a look through the cypress run docs, and it looks like the --spec
flag allows the user to specify a test to run. Additionally, --record
effectively lets you remote control the Cypress UI from the command line, which could come in handy. I'm currently writing up a couple of lines for CONTRIBUTING.md describing these flags, with a link to the documentation for the rest of the flags cypress run
accepts.
|
||
// NOTE: This response will change when Cypress integration tests are incorporated | ||
cy.contains('Error'); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something I think i failed to go over with you, there are <!-- APP_INFO -->
or <!-- EDIT_CODE -->
comments in most of the index.html files. They are triggers that we use on the backend to inject the links to edit the deployed code directly in the console.
It would be good to make sure that is captured in our tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wrote up a quick test for the verify template that uses a DOM TreeWalker
to ensure index.html only contains an EDIT_CODE
comment with no APP_INFO
. Right now it accepts more than one EDIT_CODE
; is there any case in which there will be more than one EDIT_CODE
tag?
src/index.js
Outdated
const templates = ['verify']; | ||
|
||
templates.forEach((template) => { | ||
app.use(`/${template}`, express.static(path.resolve(__dirname, '..', template, 'assets'))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it would be worth building a mini document tree walker to automatically just set each directories asset folder as available to remover the need to manually update the templates
list as more tests are written. Will probably need some light error handling to ignore possible situations where an assets
directory isn't found
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I definitely want the development server to automatically pull in templates and make them available for tests, very likely via the method you've outlined here. I'll take a swing at it in this PR, since it will pay off very soon for future tests.
@@ -4,3 +4,5 @@ coverage/ | |||
package-lock.json | |||
.env | |||
.DS_Store | |||
cypress/screenshots/ | |||
cypress/videos/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we don't actually track those videos, I wonder if by default we should just turn those off?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds like a good idea -- I'll add "video": false
to cypress.json
. I would still like to keep cypress/videos/
in .gitignore
just in case a contributor enables recording video by hand, so that video files don't end up getting accidentally committed.
cypress/fixtures/users.json
Outdated
@@ -0,0 +1,232 @@ | |||
[ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think having a README.md or a .gitkeep
file would make sense. I'd definitely get rid of the bigger fixtures
|
||
// `verify` function template UI-only tests | ||
|
||
const testNumber = "5555555555"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd go one further and use specifically a valid phone number (since the one you listed is not valid). The typical one we have in examples is: +18448144627
I'm using it in this test for example: https://github.com/twilio-labs/configure-env/blob/main/src/__tests__/validators.test.ts#L22
The development server now automatically crawls function template directories and automatically creates routes for all of the asset directories it finds.
It looks promising. Does this mean all the Cypress tests will live inside the root level Cypress folder rather than inside each individual project? |
@jmulcahey-twilio, @dkundel is there any progress on this PR? |
Description
Adds the Cypress automated browser testing framework, an
npm start
command that starts a development server to host the templates with Cypress tests, and includes UI tests for theverify
function template.Checklist
npm test
locally and it passed without errors.Related issues