-
Notifications
You must be signed in to change notification settings - Fork 341
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 #304
base: main
Are you sure you want to change the base?
solution #304
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 pull request! While your implementation is on the right track, there are a few critical issues that need to be addressed to ensure the functionality and reliability of the tests:
-
Selector Specificity: Ensure that the
cy.get
command accurately identifies the delete button. If there are multiple buttons with similar classes, a more specific selector is necessary to prevent test failures. -
Chaining with
findByPlaceholder
: ThefindByPlaceholder
command should return the result ofcy.get
to allow chaining with other Cypress commands. This is crucial for the getter methods to work as intended. -
Configuration for
assertPageURL
: TheassertPageURL
command assumes thebaseUrl
is set in the Cypress configuration. Please verify that this configuration is correctly set to avoid assertion failures. -
Handling Response in
createArticle
: Ensure that thecreateArticle
command handles the response fromcy.request
if you need to use the article data (e.g., slug) in subsequent test steps. -
Method Definitions in
ArticlePageObject
: Several methods such asassertArticleTitle
,onClickSubmitArticle
,createArticle
, andgenerateArticle
need to be defined in theArticlePageObject
. These methods should correctly interact with the page elements and assert the expected outcomes. -
Login Command: Verify that the
cy.login
command is correctly defined and logs in the user as expected.
Addressing these issues will significantly improve the robustness and reliability of your test suite. Keep up the great work, and I'm confident you'll have this polished in no time! 😊
✨ 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.
beforeEach(() => { | ||
cy.task('generateUser').then((generatedUser) => { | ||
user = generatedUser; | ||
cy.login(user.email, user.username, user.password); |
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 cy.login
command is correctly defined in your Cypress commands. If it's a custom command, verify its implementation to ensure it logs in the user as expected.
}); | ||
|
||
it('', () => { | ||
it('should fill in article and create it', () => { | ||
const article = ArticlePage.generateArticle(); |
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 generateArticle
should be defined in the ArticlePageObject
. Ensure it returns an article object with all necessary fields for creation.
it('', () => { | ||
it('should fill in article and create it', () => { | ||
const article = ArticlePage.generateArticle(); | ||
ArticlePage.createArticle(article); |
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 createArticle
should be defined in the ArticlePageObject
. Ensure it correctly interacts with the page to fill in the article details.
it('should fill in article and create it', () => { | ||
const article = ArticlePage.generateArticle(); | ||
ArticlePage.createArticle(article); | ||
ArticlePage.onClickSubmitArticle(); |
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 onClickSubmitArticle
should be defined in the ArticlePageObject
. Ensure it triggers the article submission correctly.
const article = ArticlePage.generateArticle(); | ||
ArticlePage.createArticle(article); | ||
ArticlePage.onClickSubmitArticle(); | ||
ArticlePage.assertArticleTitle(article.title); |
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 assertArticleTitle
should be defined in the ArticlePageObject
. Ensure it correctly asserts that the article title is displayed as expected.
|
||
class ArticlePageObject extends PageObject { | ||
get articleTitleField() { | ||
return cy.findByPlaceholder('Article Title'); |
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 findByPlaceholder
command returns the element to allow chaining. This is important for the getter methods to work as intended.
} | ||
|
||
get articleAboutField() { | ||
return cy.findByPlaceholder(`What's this article about?`); |
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 findByPlaceholder
command returns the element to allow chaining. This is important for the getter methods to work as intended.
} | ||
|
||
get articleBodyField() { | ||
return cy.findByPlaceholder('Write your article (in markdown)'); |
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 findByPlaceholder
command returns the element to allow chaining. This is important for the getter methods to work as intended.
} | ||
|
||
get articleTagsField() { | ||
return cy.findByPlaceholder('Enter tags'); |
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 findByPlaceholder
command returns the element to allow chaining. This is important for the getter methods to work as intended.
} | ||
|
||
get deleteArticleButton() { | ||
return cy.get('.btn.btn-outline-danger').contains('Delete Article'); |
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 cy.get
command correctly identifies the delete button. If there are multiple buttons with similar classes, consider using a more specific selector.
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.
Review Summary
Great job on your submission! 🎉 Your code meets the task requirements and constraints effectively. Here are some key points and suggestions for improvement:
-
Article Functionality Test: The test file is well-structured, but ensure that the
cy.createArticle
andcy.assertPageURL
commands are correctly implemented or imported, as they are not defined within the file itself. -
PageObject Class: This class is simple and effective for managing page visits in Cypress tests. No changes are needed here.
-
Custom Commands: The
commands.js
file is well-implemented, enhancing readability and reusability. ThecreateArticle
andassertPageURL
commands are correctly defined, addressing potential issues noted in the test file. -
ArticlePageObject Class: Extending the
PageObject
class, this class effectively manages article-related elements and uses thefaker
library for generating random data, which is a great approach for testing.
Overall, your code is correctly implemented according to the task requirements. While there are minor issues to address, such as ensuring all commands are properly defined or imported, the decision to approve is made to help you move forward with your learning. Please review the comments to improve your code, and remember to address any critical issues in the future to avoid similar problems. 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! ✨
|
||
it('should be able to delete an article', () => { | ||
const article = ArticlePage.generateArticle(); | ||
cy.createArticle(article).then((response) => { |
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 cy.createArticle
command is used here, but it is not defined within this file. Ensure that this command is implemented elsewhere in your Cypress commands or imported correctly.
cy.visit(`/article/${response.body.article.slug}`); | ||
ArticlePage.assertArticleTitle(article.title); | ||
cy.get('.btn.btn-outline-danger.btn-sm').eq(0).click(); | ||
cy.assertPageURL('/'); |
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 cy.assertPageURL
command is used here, but it is not defined within this file. Ensure that this command is implemented elsewhere in your Cypress commands or imported correctly.
No description provided.