[TESTING] Survey view testing #4802
Conversation
tests/cypress/e2e/for-teacher_page/class_page/class-survey.cy.js
Outdated
Show resolved
Hide resolved
| cy.wait(500); | ||
|
|
||
| cy.get(".view_class").first().click() | ||
| cy.get("body").then(surveyView).then(survey => { |
There was a problem hiding this comment.
This works, but is not really the Cypress way of doing it. You don't need to target the body and then find the survey, cy.get can select any element of the page, so doing cy.get('#survey') will do the job. Look at how we target elements on the other tests for some guidance
There was a problem hiding this comment.
The thing is, if a class doesn't contain students, there is no survey view, so cy.get('#survey') fails. So I was looking for a workaround and found this in other tests.
There was a problem hiding this comment.
Ohh I get it. CLASS1 in the database has some students, so maybe targetting that class is the way to go. Otherwise, you might create a new class, add some students and then run this test to guarantee that the survey will be shown.
There was a problem hiding this comment.
That's a good one! Will do that thanks.
| loginForTeacher(); | ||
|
|
||
| cy.get(".view_class").first().click() | ||
| cy.get("body").then(surveyView).then(survey => { |
There was a problem hiding this comment.
These tests are dependent on one another. This is not advisable unless on specific circumstances where we want to save some time, but we should avoid them as much as possible. Our philosophy with Cypress is making the tests independent from one another. See here: https://docs.cypress.io/guides/core-concepts/writing-and-organizing-tests#Test-Isolation
There was a problem hiding this comment.
Hi, Annelein! Good job with the testing, but I think we can improve them a bit:
-
As I told you in the comment, we don't need to target
bodyto get an element, just as you can do in JQuery or Javascript you can target an element by its id. We also have a function, calledcy.getBySelwhere we don't even target by id, but an specefic target dedicated for testing. You can see an example of that in action in theclass_customization.cy.jsfile. -
It's preferable for the tests to be independent on one another. This can take a hit in our performance and make the test suite longer, but I think is best we stick with this for now. What does this mean for this particular PR? For example, if you're checking if by only answering one question, the next time you come to the class you'll be shown three question, you do that in a single test.
For this you can for example create a new class before each tests, with a beforeEach hook.
I recommend you take a quick peek to Cypress documentation to know a little bit about the framework: https://docs.cypress.io/guides/core-concepts/introduction-to-cypress
|
@jpelay let me know if this is better! |
jpelay
force-pushed
the
survey-view-testing
branch
from
83706f2
to
9661946
Compare
|
Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork). |
Fixes #4782
Tests:
It would be nice to only test classes that have students, anyone knows how to do that?