Chore: Adding Cypress tests to cover existing functional tests #912
Conversation
ConradJChan
commented
Feb 4, 2019
ConradJChan
commented
- Remove existing functional tests
- Add README for running cypress tests
|
Verified that @ConradJChan has signed the CLA. Thanks for the pull request! |
src/index.html
Outdated
| @@ -93,6 +93,7 @@ | |||
| /* global Box */ | |||
| var preview = new Box.Preview(); | |||
| preview.show(fileid, token, { | |||
| ...options, | |||
There was a problem hiding this comment.
These files aren't transpiled by Babel, so I believe this will fail in IE11.
| const token = Cypress.env('ACCESS_TOKEN'); | ||
| const fileId = Cypress.env('FILE_ID_DOC'); | ||
|
|
||
| /* eslint-disable */ |
There was a problem hiding this comment.
What rules are we breaking here?
There was a problem hiding this comment.
I think jsdoc comments for the functions
| const showControls = () => { | ||
| // Hover over the preview to trigger the controls | ||
| cy.getByTestId('bp').trigger('mouseover'); | ||
| // Assert that the controls are shown |
There was a problem hiding this comment.
I'm a fan of code comments, but this logic seems pretty self-explanatory to me. Is a comment needed here?
| cy.contains('The Content Platform for Your Apps'); | ||
| // Get the current dimensions of the page | ||
| getPage(1).then(($page) => { | ||
| cy.wrap($page[0].scrollWidth).as('origWidth'); |
There was a problem hiding this comment.
Generally want to avoid abbreviation where possible.
|
|
||
| showControls(); | ||
| // Click the zoom out button | ||
| cy.getByTestId('Zoom out').click(); |
There was a problem hiding this comment.
Seems like a good use case for getByTitle.
| showControls(); | ||
| cy.getByTestId('Click to enter page number').click(); | ||
| cy.getByTestId('page-num-input').should('be.visible').type('2').blur(); | ||
| // cy.getByTestId('page-num-input').type('2').blur(); |
There was a problem hiding this comment.
Not sure if this is intentional, but try to avoid commented code.
| }; | ||
|
|
||
| /* eslint-disable */ | ||
| const getPage = (pageNum) => cy.get(`.page[data-page-number=${pageNum}]`); |
There was a problem hiding this comment.
If this is used in more than one file, we should probably share it between them somehow.
| it('Should handle page navigation via buttons', () => { | ||
| getPage(1).should('be.visible'); | ||
| cy.contains('The Content Platform for Your Apps'); | ||
| cy.getByTestId('current-page').invoke('text').should('equal', '1'); |
There was a problem hiding this comment.
Minor: You can alias these elements for reuse later in the test rather than selecting them each time.
| cy.getByTestId('current-page').invoke('text').should('equal', '1'); | ||
|
|
||
| showControls(); | ||
| cy.getByTestId('Next page').click(); |
There was a problem hiding this comment.
I think we should probably use contains or getByTitle for any natural language selectors, ideally.
Merge branch 'cypress-migration' of github.com:ConradJChan/box-content-preview into cypress-migration
|
|
||
| beforeEach(() => { | ||
| cy.server(); | ||
| cy.route('GET', '**/files/*?fields=download_url', { download_url: '' }); |
There was a problem hiding this comment.
Should probably be an .int.js test if we're stubbing the endpoints.
