added tests #525
added tests #525
Conversation
There was a problem hiding this comment.
Thank you for your hard work on this task! There are a few critical issues that need to be addressed before we can approve the pull request:
-
findByIdCommand: The custom commandfindByIdis missing areturnstatement, which is crucial for enabling command chaining in Cypress. Please addreturn cy.get(#${id})to ensure it functions as intended. -
Subject Field: The
generateUserfunction generates asubjectfield, but it is not currently used in the test script. Make sure to include this field in the form filling process to meet the task requirements. -
defaultCommandTimeout: ThedefaultCommandTimeoutis set to an unusually high value of 100000 milliseconds. This could lead to unnecessarily long wait times during test execution. Consider reducing it to a more standard value, such as 4000 or 5000 milliseconds, unless there's a specific need for the high timeout.
Once these issues are resolved, your code will be in great shape! Keep up the excellent work, and don't hesitate to reach out if you have any questions. 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.
cypress.config.js
Outdated
| } | ||
| }, | ||
| baseUrl: 'https://demoqa.com', | ||
| defaultCommandTimeout: 100000 |
There was a problem hiding this comment.
The defaultCommandTimeout is set to 100000 milliseconds (100 seconds), which is quite high. This might lead to unnecessarily long wait times during test execution. Consider setting it to a more reasonable value, such as 4000 or 5000 milliseconds, unless there is a specific reason for this high timeout.
| currentAddress, | ||
| state, | ||
| city | ||
| } = generateUser(); |
There was a problem hiding this comment.
The generateUser function also returns a subject field, but it is not used in the test. According to the task, all fields should be filled. Make sure to include the subject field in the form filling process.
cypress/support/commands.js
Outdated
| @@ -23,3 +23,6 @@ | |||
| // | |||
| // -- This will overwrite an existing command -- | |||
| // Cypress.Commands.overwrite('visit', (originalFn, url, options) => { ... }) | |||
| Cypress.Commands.add('findById', (id) => { | |||
| cy.get(`#${id}`); | |||
There was a problem hiding this comment.
The findById command should return the result of cy.get(#${id}) to allow for command chaining. Add a return statement before cy.get to ensure the command can be used in a chain.
There was a problem hiding this comment.
Great job on automating the student registration form workflow! 🎉 Your code meets the task requirements by correctly visiting the URL, filling out the form, submitting it, and verifying the modal window. However, there are a few areas to improve:
- Selectors: Ensure the
submitbutton's ID is correctly referenced. If it lacks an ID, consider another selector strategy. Also, verify that.subjects-auto-complete__placeholdertargets the correct input field. - Custom Command: Double-check the
findByIdimplementation and ensure the IDs used match the actual HTML IDs. - Node Events: The
setupNodeEventsfunction is empty. Implement any necessary event listeners or leave it if none are needed.
These adjustments will enhance your code's reliability. Keep up the great work, and continue learning from these experiences! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| @@ -3,6 +3,8 @@ const { defineConfig } = require('cypress'); | |||
| module.exports = defineConfig({ | |||
| e2e: { | |||
| setupNodeEvents(on, config) { | |||
| } | |||
| }, | |||
There was a problem hiding this comment.
The setupNodeEvents function is currently empty. If you intend to use any node event listeners, you should implement them here. If not, you can leave it as is.
| cy.findById('firstName').type(userName); | ||
| cy.findById('lastName').type(userName); | ||
| cy.findById('userEmail').type(email); | ||
| cy.findById('userNumber').type(mobile); | ||
| cy.findById('currentAddress').type(currentAddress); | ||
| cy.findById('state').type(state); | ||
| cy.findById('city').type(city); |
There was a problem hiding this comment.
The findById custom command is used here, but it seems to be incorrectly implemented. The findById command should be defined to select elements by their ID, but the IDs used here (e.g., 'firstName', 'lastName', etc.) should be verified to ensure they match the actual IDs in the HTML. Make sure the IDs are correct or adjust the command accordingly.
| cy.findById('currentAddress').type(currentAddress); | ||
| cy.findById('state').type(state); | ||
| cy.findById('city').type(city); | ||
| cy.get('.subjects-auto-complete__placeholder').type(subject); |
There was a problem hiding this comment.
The selector .subjects-auto-complete__placeholder might not be the best choice if it targets a placeholder element. Ensure this selector targets an input field where the subject can be typed.
| cy.get(`input[id='hobbies-checkbox-1']`).check(); | ||
| cy.get(`input[id='hobbies-checkbox-1']`).should('be.checked'); | ||
|
|
||
| cy.get(`button[id='submit']`).click(); |
There was a problem hiding this comment.
Ensure that the submit button's ID is correctly referenced. If the button does not have an ID, consider using a different selector strategy.
No description provided.