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

Solution #264

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Solution #264

wants to merge 2 commits into from

Conversation

vStryhun
Copy link

No description provided.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

The email generation logic could potentially cause issues with special characters or spaces in firstName, which is critical for functionality. Additionally, there are multiple typos in the test descriptions that should be corrected. Please address these issues to ensure the code meets the necessary standards. Keep up the good work! 😊

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

cy.visit('/webtables');
});

it('shoud can change rows count', () => {

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 test description. 'shoud' should be corrected to 'should'.

cy.get('[aria-label="rows per page"]').should('contain', '5 rows');
});

it('shoud add a new worker', () => {

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 test description. 'shoud' should be corrected to 'should'.

cy.addNewWorker();
});

it('shoud navigate to the next page by pagination', () => {

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 test description. 'shoud' should be corrected to 'should'.

cy.contains('.-btn', 'Previous').click();
});

it('shoud find a worker in the search field and edit it', () => {

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 test description. 'shoud' should be corrected to 'should'.

cy.contains('[role=gridcell]', 'Lorrem').should('be.visible');
});

it('shoud find a worker in the search field by last name', () => {

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 test description. 'shoud' should be corrected to 'should'.

cy.get('#searchBox').type('[email protected]');
});

it('shoud find a worker in the search field by salary', () => {

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 test description. 'shoud' should be corrected to 'should'.

cy.get('#searchBox').type('10000');
});

it('shoud find a worker in the search field by department', () => {

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 test description. 'shoud' should be corrected to 'should'.

cy.get('#searchBox').type('Insurance');
});

it('shoud delete a worker', () => {

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 test description. 'shoud' should be corrected to 'should'.

cy.contains('[role=gridcell]', 'Cierra').should('not.exist');
});

it('shoud delete all workers', () => {

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 test description. 'shoud' should be corrected to 'should'.

const randomNumber = Math.random().toString().slice(4);
const firstName = faker.person.firstName() + randomNumber;
const lastName = faker.person.lastName();
const userEmail = `${firstName}@gmail.com`;

Choose a reason for hiding this comment

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

The email generation logic might cause issues if firstName contains special characters or spaces. Consider using a more reliable method to ensure valid email addresses.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Great job on setting up the configuration file for Cypress! 🎉 While the tests are functional, consider adding assertions to verify outcomes and improve reliability. Addressing these will enhance your testing skills. Keep up the good work and learn from the feedback!

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

cy.visit('/webtables');
});

it('should can change rows count', () => {

Choose a reason for hiding this comment

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

The test description 'should can change rows count' is grammatically incorrect. Consider changing it to 'should change rows count'.

cy.get('[aria-label="rows per page"]').should('contain', '5 rows');
});

it('should add a new worker', () => {

Choose a reason for hiding this comment

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

The test 'should add a new worker' lacks assertions to verify that the worker was actually added. Consider adding assertions to check the presence of the new worker in the table.

cy.contains('[role=gridcell]', 'Lorrem').should('be.visible');
});

it('should find a worker in the search field by last name', () => {

Choose a reason for hiding this comment

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

The test 'should find a worker in the search field by last name' lacks assertions to verify that the worker is found. Consider adding assertions to check the presence of the worker in the table.

cy.get('#searchBox').type('Cantrell');
});

it('should find a worker in the search field by age', () => {

Choose a reason for hiding this comment

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

The test 'should find a worker in the search field by age' lacks assertions to verify that the worker is found. Consider adding assertions to check the presence of the worker in the table.

cy.get('#searchBox').type('45');
});

it('should find a worker in the search field by email', () => {

Choose a reason for hiding this comment

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

The test 'should find a worker in the search field by email' lacks assertions to verify that the worker is found. Consider adding assertions to check the presence of the worker in the table.

cy.get('#searchBox').type('[email protected]');
});

it('should find a worker in the search field by salary', () => {

Choose a reason for hiding this comment

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

The test 'should find a worker in the search field by salary' lacks assertions to verify that the worker is found. Consider adding assertions to check the presence of the worker in the table.

cy.get('#searchBox').type('10000');
});

it('should find a worker in the search field by department', () => {

Choose a reason for hiding this comment

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

The test 'should find a worker in the search field by department' lacks assertions to verify that the worker is found. Consider adding assertions to check the presence of the worker in the table.

cy.get('#searchBox').type('Insurance');
});

it('should delete a worker', () => {

Choose a reason for hiding this comment

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

The test 'should delete a worker' lacks assertions to verify that the correct worker was deleted. Consider adding assertions to ensure the worker 'Alden' is no longer present.


function generateWorker () {
const randomNumber = Math.random().toString().slice(4);
const firstName = faker.person.firstName() + randomNumber;

Choose a reason for hiding this comment

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

The firstName is concatenated with a random number. This might be intentional for uniqueness, but if not, consider removing the random number to keep the name realistic.

const firstName = faker.person.firstName() + randomNumber;
const lastName = faker.person.lastName();
const userEmail = faker.internet.email();
const age = faker.number.int({ max: 65 });

Choose a reason for hiding this comment

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

The age is generated with a maximum value of 65. If there is a minimum age requirement, consider specifying it in the options.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants