-
Notifications
You must be signed in to change notification settings - Fork 357
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
solution #280
base: main
Are you sure you want to change the base?
solution #280
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your hard work on this project! 🚀 While your implementation is mostly solid and meets the task requirements, there are a few critical issues that need to be addressed before we can approve the pull request:
-
Typographical Errors:
- There's a typo in the method name
verifyAricleCreation
which should be corrected toverifyArticleCreation
. This typo appears multiple times and needs to be corrected throughout the codebase to ensure consistency and functionality. - Similarly, the method
typeArticleTags
referencesthis.articleTagField
, which should bethis.articleTagsField
to match the getter method defined above.
- There's a typo in the method name
-
CSS Selector Issues:
- The selector
class="btn btn-sm btn-outline-secondary action-btn"
is incorrect. It should be.btn.btn-sm.btn-outline-secondary.action-btn
to correctly select the element by class names. This is crucial for the tests to interact with the correct elements on the page.
- The selector
These issues are critical as they directly impact the functionality of your tests. Once these are addressed, your code will be in great shape! Keep up the great work, and I'm looking forward to seeing the improvements. 😊
✨ 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/e2e/article.cy.js
Outdated
|
||
articlePage.verifyAricleCreation(articleTitle); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a typo in the method name verifyAricleCreation
. It should be verifyArticleCreation
. Ensure that the method is correctly defined in the ArticlePageObject
class.
cypress/e2e/article.cy.js
Outdated
|
||
articlePage.clickPublishBtn(); | ||
|
||
articlePage.verifyAricleCreation(articleTitle); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a typo in the method name verifyAricleCreation
. It should be verifyArticleCreation
. Ensure that the method is correctly defined in the ArticlePageObject
class.
cypress/e2e/user.cy.js
Outdated
it.skip('should be able to follow the another user', () => { | ||
it('should be able to follow/unfollow the another user', () => { | ||
cy.visit('#/@riot'); | ||
cy.get('class="btn btn-sm btn-outline-secondary action-btn"').click(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The selector class="btn btn-sm btn-outline-secondary action-btn"
is incorrect. It should be .btn.btn-sm.btn-outline-secondary.action-btn
to correctly select the element by class names.
cypress/e2e/user.cy.js
Outdated
|
||
cy.get('.btn').should('contain', 'Unfollow'); | ||
|
||
cy.get('class="btn btn-sm btn-outline-secondary action-btn"').click(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The selector class="btn btn-sm btn-outline-secondary action-btn"
is incorrect. It should be .btn.btn-sm.btn-outline-secondary.action-btn
to correctly select the element by class names.
return cy.get('.container'); | ||
} | ||
|
||
verifyAricleCreation(articleTitle) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a typo in the method name verifyAricleCreation
. It should be verifyArticleCreation
.
} | ||
|
||
typeArticleTags(articleTag) { | ||
this.articleTagField.clear().type(`${articleTag}{enter}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method typeArticleTags
references this.articleTagField
, which seems to be incorrect. It should be this.articleTagsField
to match the getter method defined above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work on implementing the Cypress tests using the Page Object Model! 🎉 Your code meets the task requirements and constraints, effectively covering functionalities such as creating, editing, and deleting articles, signing in and signing up with various credentials, and user interactions like following and unfollowing.
However, there are some critical issues to address to ensure the robustness of your tests:
-
Cypress Commands: Ensure that
cy.register()
andcy.login()
are correctly defined in your Cypress support files. Without these, tests will fail when attempting to register or log in users. -
API Endpoints: Double-check the API endpoints used in your custom commands, especially for login operations. Typically, login endpoints might be
/api/login
or/api/auth/login
, and theusername
parameter is usually not required. -
Verification Methods: Enhance methods like
assertSignUpFailed
,assertLoginFailed
,verifyLogout
,verifyProfilePage
, andverifyDeletedArticle
to accurately check for expected outcomes, such as error messages or the absence of specific elements.
Addressing these issues will improve the reliability of your tests. Keep up the great work, and use this feedback to refine your code further! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
cy.register(); | ||
cy.login(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that cy.register()
and cy.login()
are defined in your Cypress support commands. If these commands are not implemented, the tests will fail when trying to register and log in a user.
|
||
articlePage.verifyDeletedArticle(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The verifyDeletedArticle
method should accurately verify that the article has been deleted. Ensure this method checks the absence of the article in the list or any other appropriate verification.
cy.register(); | ||
cy.login(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that cy.register()
and cy.login()
are defined in your Cypress support commands. If these commands are not implemented, the tests will fail when trying to register and log in a user.
|
||
settingsPage.editUsername(user.username); | ||
settingsPage.submitEdit(); | ||
settingsPage.verifyProfilePage(user.username); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The verifyProfilePage
method should accurately verify that the username has been updated. Ensure this method checks the displayed username on the profile page.
}); | ||
|
||
it('should provide an ability to log out', () => { | ||
|
||
settingsPage.logoutUser(); | ||
settingsPage.verifyLogout(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The verifyLogout
method should accurately verify that the user has been logged out. Ensure this method checks for the absence of user-specific elements or the presence of a login prompt.
|
||
signUpPage.clickSignUpBtn(); | ||
|
||
signUpPage.assertSignUpFailed(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The assertSignUpFailed
method should accurately verify that the sign-up attempt with a wrong email has failed. Ensure this method checks for an error message or any other indication of a failed sign-up.
|
||
signUpPage.assertSignUpFailed(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The assertSignUpFailed
method should accurately verify that the sign-up attempt with a wrong password has failed. Ensure this method checks for an error message or any other indication of a failed sign-up.
cy.register(); | ||
cy.login(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that cy.register()
and cy.login()
are defined in your Cypress support commands. If these commands are not implemented, the tests will fail when trying to register and log in a user.
}); | ||
|
||
Cypress.Commands.add('login', | ||
(email = '[email protected]', username = 'riot', password = '12345Qwert!') => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The username
parameter is not typically required for login operations. Consider removing it from the login
command unless your backend specifically requires it.
|
||
Cypress.Commands.add('login', | ||
(email = '[email protected]', username = 'riot', password = '12345Qwert!') => { | ||
cy.request('POST', '/api/users', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that the API endpoint /api/users
is correct for the login operation. Typically, login endpoints might be something like /api/login
or /api/auth/login
.
No description provided.