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

Chore: Add support for integration testing with Cypress #909

Merged
merged 8 commits into from
Jan 31, 2019

Conversation

ConradJChan
Copy link
Contributor

No description provided.

@boxcla
Copy link

boxcla commented Jan 30, 2019

Verified that @ConradJChan has signed the CLA. Thanks for the pull request!

jstoffan
jstoffan previously approved these changes Jan 30, 2019
Copy link
Collaborator

@jstoffan jstoffan left a comment

Choose a reason for hiding this comment

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

What does the migration look like for the existing tests? Will that be done as a near-future follow-up? Otherwise we have two end-to-end test suites, which isn't ideal.

.travis.yml Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
test/support/commands.js Outdated Show resolved Hide resolved
@ConradJChan
Copy link
Contributor Author

@jstoffan Yeah, this was just to get the groundwork in place. Was thinking of creating an on call task to migrate the existing functional tests over to cypress, then we can get rid of the old functional tests.

jstoffan
jstoffan previously approved these changes Jan 30, 2019
jstoffan
jstoffan previously approved these changes Jan 30, 2019
it('Should load a document preview', () => {
const fileId = Cypress.env('FILE_ID_DOC');
cy.showPreview(token, fileId);
cy.getByTestId('preview-container').contains('The Content Platform for Your Apps', { timeout: 10000 });
Copy link
Collaborator

@jstoffan jstoffan Jan 30, 2019

Choose a reason for hiding this comment

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

Is the timeout needed to account for fetching the file/rep? You should be able to use a combination of cy.route and cy.wait to avoid an arbitrary timeout.

You could also wait for the load event from the Preview object, which would be even better, since it hides the implementation details of file/rep fetching. Props to @jeremypress for that idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried the load event approach, but couldn't find a way to have Cypress wait for additional assertions within the 'load' event handler. Instead I wait for the .bp container to have the .bp-loaded class before attempting to assert the content

let token;

beforeEach(() => {
token = Cypress.env('ACCESS_TOKEN');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like this could be made a const and done just inside describe above unless token is modified in tests.

// Show the preview
cy.showPreview(token, fileId);
// Wait for .bp to load viewer
cy.getByTestId('bp').should('have.class', 'bp-loaded')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this live in the showPreview helper?

Cypress.Commands.add('getByTestId', (testId) => cy.get(`[data-testid="${testId}"]`));
Cypress.Commands.add('getByTitle', (title) => cy.get(`[title="${title}"]`));
Cypress.Commands.add('showPreview', (token, fileId) => {
cy.get('[data-testid="token"]').type(token);
Copy link
Collaborator

Choose a reason for hiding this comment

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

These should be able to use cy.getByTestId since it's declared above.

@ConradJChan ConradJChan merged commit 68e962e into box:master Jan 31, 2019
@ConradJChan ConradJChan deleted the cypress branch January 31, 2019 01:42
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