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 #305

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

Conversation

JedrzejewskiJakub
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.


Thank you for your hard work on this project! While your implementation is mostly solid and meets many of the task requirements, there are a few critical issues that need to be addressed before we can approve the pull request:

  1. Selector Conflict: The use of the .lead class for multiple elements (customerName, customerCard, and orderId) could lead to incorrect element selection. Please ensure that each selector uniquely identifies the intended element or consider using more specific selectors to avoid potential conflicts.

  2. Import Statement Issue: There is a missing from keyword in the import statement for HomeAndCataloguePageObject. This needs to be corrected to ensure the code functions as expected.

  3. Random Number Generation: The Math.random() function is being used incorrectly. It does not accept any arguments. Please update the logic to generate random numbers correctly, such as using Math.ceil(Math.random() * 1000) for a random number between 1 and 1000.

  4. Typo in Test Description: There is a typo in the test description where 'purchuase' should be corrected to 'purchase'. While this is a minor issue, it's important for clarity and professionalism.

Once these issues are resolved, your code will be in great shape! Keep up the great work, and feel free to reach out if you have any questions or need further assistance. 😊

✨ 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.


module.exports = defineConfig({
e2e: {
baseUrl: 'https://www.demoblaze.com/',
baseUrl: 'https://www.demoblaze.com',
setupNodeEvents(on, config) {
on('task', {
generateUser() {
const randomNumber = Math.ceil(Math.random(1000) * 1000);

Choose a reason for hiding this comment

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

The Math.random(1000) function does not accept any arguments. To generate a random number between 1 and 1000, you should use Math.ceil(Math.random() * 1000).

setupNodeEvents(on, config) {
on('task', {
generateUser() {
const randomNumber = Math.ceil(Math.random(1000) * 1000);
const randomMonth = Math.ceil(Math.random(1000) * 12);

Choose a reason for hiding this comment

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

The Math.random(1000) function does not accept any arguments. To generate a random month between 1 and 12, you should use Math.ceil(Math.random() * 12).

setupNodeEvents(on, config) {
on('task', {
generateUser() {
const randomNumber = Math.ceil(Math.random(1000) * 1000);
const randomMonth = Math.ceil(Math.random(1000) * 12);
const randomYear = Math.ceil(Math.random(1000) * 2024);

Choose a reason for hiding this comment

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

The Math.random(1000) function does not accept any arguments. To generate a random year between 1 and 2024, you should use Math.ceil(Math.random() * 2024).

@@ -1,11 +1,52 @@
import PlaceOrderFormPageObject from '../support/pages/orderForm.pageObject';
import HomeAndCataloguePageObject

Choose a reason for hiding this comment

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

There seems to be a missing from keyword in the import statement for HomeAndCataloguePageObject. Ensure that the import statement is correctly formatted.

});

it('', () => {

it('should be placed by purchuase the form', () => {

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: 'purchuase' should be corrected to 'purchase'.

@@ -1,6 +1,6 @@
import ContactFormPageObject from '../support/pages/contactForm.pageObject';
import HomeAndCataloguePageObject

Choose a reason for hiding this comment

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

There seems to be a missing from keyword in the import statement for HomeAndCataloguePageObject. Ensure that the import statement is correctly formatted.

}

get customerName() {
return cy.get('.lead');

Choose a reason for hiding this comment

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

The selector .lead is used for customerName, customerCard, and orderId. Ensure that this selector uniquely identifies the intended element, or consider using more specific selectors to avoid potential conflicts.

}

get customerCard() {
return cy.get('.lead');

Choose a reason for hiding this comment

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

The selector .lead is used for customerName, customerCard, and orderId. Ensure that this selector uniquely identifies the intended element, or consider using more specific selectors to avoid potential conflicts.

}

get orderId() {
return cy.get('.lead');

Choose a reason for hiding this comment

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

The selector .lead is used for customerName, customerCard, and orderId. Ensure that this selector uniquely identifies the intended element, or consider using more specific selectors to avoid potential conflicts.

Copy link
Author

@JedrzejewskiJakub JedrzejewskiJakub left a comment

Choose a reason for hiding this comment

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

fixed

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.

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

Something went wrong while processing the review.
[CODE: 3]

If the issue is within your control, please address it.
The next review attempt will automatically launch in 1 hour.
If you believe this is an error or need assistance, please contact the support team.

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.

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

Something went wrong while processing the review.
[CODE: 3]

If the issue is within your control, please address it.
The next review attempt will automatically launch in 1 hour.
If you believe this is an error or need assistance, please contact the support team.

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.

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

Something went wrong while processing the review.
[CODE: 3]

If the issue is within your control, please address it.
The next review attempt will automatically launch in 1 hour.
If you believe this is an error or need assistance, please contact the support team.

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