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

CC regression - accordion feature #334

Merged
merged 25 commits into from
Apr 29, 2024
Merged

Conversation

sigadamvenkata
Copy link
Collaborator

sigadamvenkata and others added 22 commits January 22, 2024 17:11
CC Merchtable block automation (JIra: MWPW-140978)
CC sticky promo bar, promo with close action automated test cases
Jira : MWPW-140982, MWPW-142252
…eatures

CC Regression automated cases for media rounded corners, breadcrumb features (MWPW-140976, MWPW-142543)
removed the lengthy URL
removed tailing spaces from given lines
@JackySun9
Copy link
Contributor

@sigadamvenkata could you fix the eslint warnings?

@Dli3
Copy link
Contributor

Dli3 commented Apr 23, 2024

@sigadamvenkata Do you only run your tests on chrome?

@Dli3
Copy link
Contributor

Dli3 commented Apr 23, 2024

Does CC have their own unique accordion block? @sigadamvenkata

Copy link
Collaborator

@amauch-adobe amauch-adobe left a comment

Choose a reason for hiding this comment

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

Looks really good, like everyone else said handle the ESLint errors, I also added a small grammar fix and a recommendation to do instead of having a waitfor timeout call before checking the url link switch.

test.beforeEach(async ({ page }) => {
accordion = new Accordion(page);
});
// erify accordion showing up with authored question and answers and UI
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing 'V' in Verify within comment.

await accordion.accordionQuestion1.click();
expect(await accordion.accordexpanded).toBeTruthy();
await accordion.firstQuestionLink.click();
await page.waitForTimeout(1000);
Copy link
Collaborator

Choose a reason for hiding this comment

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

As a recommendation I would look for a way to wait for an element on the subsequent page or it's load state to finish before checking the url since waiting for timeouts aren't very reliable since network or CI/CD pipeline issues could cause it to take longer than just a second.

@JackySun9
Copy link
Contributor

@sigadamvenkata if you clone this from milo to cc, we might no need to do this, are you cloning the tests from milo?

Copy link
Contributor

@skumar09 skumar09 left a comment

Choose a reason for hiding this comment

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

Approving,
Please fix the comments from @amauch-adobe

@Dli3 Dli3 added the run-nala Run Nala Test Automation against PR/Branch label Apr 25, 2024
corrected the typo
fixed indentations issue at line 14,15,16

Also removed wait from line number 81
@sigadamvenkata
Copy link
Collaborator Author

@amauch-adobe made the changes mentioned by you.

@skumar09 @JackySun9 @Dli3

@amauch-adobe amauch-adobe self-requested a review April 29, 2024 16:42
Copy link
Collaborator

@amauch-adobe amauch-adobe left a comment

Choose a reason for hiding this comment

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

Approved. Thanks for addressing the fixes.

@sigadamvenkata sigadamvenkata merged commit a0b6b56 into adobecom:main Apr 29, 2024
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-nala Run Nala Test Automation against PR/Branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants