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

193 Mock Test Auth & 2 tests from 192 #197

Merged
merged 16 commits into from
Feb 16, 2023
Merged

193 Mock Test Auth & 2 tests from 192 #197

merged 16 commits into from
Feb 16, 2023

Conversation

summer-cook
Copy link
Contributor

@summer-cook summer-cook commented Feb 10, 2023

Summary

  • set up a mock auth for next auth in the test suite so Cypress tests are able to log in
  • uses cypress-dotenv plugin to be able to use environment variables from env.local file
  • updates the "home" test to be browse since it was navigating to the browse page

Related

Closes #193 e2e testing: setup reusable mock login/session for test environment#193
completes 2/3 tests on #192

Acceptance Criteria

  • As a developer, I can write tests for logged in pages

Resources

nextauthjs/next-auth#2053 (comment)

@summer-cook summer-cook marked this pull request as ready for review February 10, 2023 21:00
cypress.config.js Outdated Show resolved Hide resolved
cypress.config.js Outdated Show resolved Hide resolved
cypress/e2e/browse.cy.js Outdated Show resolved Hide resolved
cypress/support/commands.js Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
cypress/e2e/browsing.cy.js Outdated Show resolved Hide resolved
cypress/e2e/browsing.cy.js Show resolved Hide resolved
cypress/e2e/requests.cy.js Outdated Show resolved Hide resolved
cypress/e2e/requests.cy.js Outdated Show resolved Hide resolved
cypress/e2e/requests.cy.js Outdated Show resolved Hide resolved
cy.log("Successfully logged in and viewing requests page");
});
})
});
Copy link
Member

Choose a reason for hiding this comment

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

now that the wcl has been updated to display text when we have no requests, can that test be added too?

Copy link
Contributor Author

@summer-cook summer-cook Feb 15, 2023

Choose a reason for hiding this comment

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

i was going to do that in a separate PR because i needed to figure out how and it was going to take a while. Since the e2e tests look at the db itself and use the real backend, and I already have requests, i also will not be able to check if a test like that is working.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nvm, went ahead and included a check for this, but this is just checking that either of these elements exist when the user is signed in vs. saying that it needs to know whether or not a user has requests on their request list or not.

i think if we wanted to do the latter we would need to test the data itself, which isn't done with cypress out of the box but theres an add on called cypress-react-selector where we could potentially be able to do something like this.
heres an example of how it could be used: https://glebbahmutov.com/blog/react-state-from-e2e-tests/

but this is more in depth and should probably be a separate ticket

Copy link
Member

Choose a reason for hiding this comment

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

is mock data not an option?

the current implementation is ok for now, but it should be expanded in the future. although the tests says it's checking for the existence of requests or a "no requests" message....it isn't. since we don't have a use case where tests exist, the "no requests" route is the only one that will ever return true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is a way with cypress fixtures - if we did that and only wanted to use mock data, we would need to mock every single api call- this is an example of that: https://blog.bitsrc.io/mocking-http-calls-in-cypress-end-to-end-tests-fa2e6b7caaf7

it can be done but it isn't really in the scope of this ticket

cypress/support/commands.js Outdated Show resolved Hide resolved
cypress/support/commands.js Outdated Show resolved Hide resolved
cypress/e2e/requests.cy.js Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
cy.log("Successfully logged in and viewing requests page");
});
})
});
Copy link
Member

Choose a reason for hiding this comment

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

is mock data not an option?

the current implementation is ok for now, but it should be expanded in the future. although the tests says it's checking for the existence of requests or a "no requests" message....it isn't. since we don't have a use case where tests exist, the "no requests" route is the only one that will ever return true.

Comment on lines +19 to +20
const requestListExists = cy.get('article.request-item').should('exist') ||
cy.get('p.no-requests').contains('You do not have any requests yet.')
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.

for now. I've unchecked the top box on the ticket under this section (seen below) so we don't lose track that we need to handle this properly.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is a way with cypress fixtures - if we did that and only wanted to use mock data, we would need to mock every single api call- this is an example of that: https://blog.bitsrc.io/mocking-http-calls-in-cypress-end-to-end-tests-fa2e6b7caaf7

it can be done but it isn't really in the scope of this ticket

Co-authored-by: Alisha Evans <[email protected]>
@summer-cook summer-cook merged commit c20fa3e into main Feb 16, 2023
@summer-cook summer-cook deleted the 193-mock-auth-tests branch March 9, 2023 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants