Skip to content

Conversation

@uroslates
Copy link
Contributor

Closes CXSPA-10033

@uroslates uroslates requested a review from a team as a code owner May 20, 2025 08:51
@github-actions github-actions bot marked this pull request as draft May 20, 2025 08:51
@uroslates uroslates marked this pull request as ready for review May 21, 2025 06:43
@github-actions
Copy link
Contributor

Merge Checks Failed

Please push a commit to re-trigger the build.
To push an empty commit you can use `git commit --allow-empty -m "Trigger Build"`

@cypress
Copy link

cypress bot commented May 21, 2025

spartacus    Run #49048

Run Properties:  status check passed Passed #49048  •  git commit cc875d52b3 ℹ️: Merge bea9d573bfb61f24aeb62e04eb99c435314ec924 into ab177a9bce64858b4e9f4b0460e5...
Project spartacus
Branch Review feature/CXSPA-10033-b2b-mycompany
Run status status check passed Passed #49048
Run duration 03m 47s
Commit git commit cc875d52b3 ℹ️: Merge bea9d573bfb61f24aeb62e04eb99c435314ec924 into ab177a9bce64858b4e9f4b0460e5...
Committer Uros Lates
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 3
Tests that did not run due to a developer annotating a test with .skip  Pending 2
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 99
View all changes introduced in this branch ↗︎

@github-actions
Copy link
Contributor

Merge Checks Failed

Please push a commit to re-trigger the build.
To push an empty commit you can use `git commit --allow-empty -m "Trigger Build"`

1 similar comment
@github-actions
Copy link
Contributor

Merge Checks Failed

Please push a commit to re-trigger the build.
To push an empty commit you can use `git commit --allow-empty -m "Trigger Build"`

@github-actions github-actions bot marked this pull request as draft May 21, 2025 07:40
@uroslates uroslates marked this pull request as ready for review May 21, 2025 11:20
@github-actions
Copy link
Contributor

Merge Checks Failed

Please push a commit to re-trigger the build.
To push an empty commit you can use `git commit --allow-empty -m "Trigger Build"`

@github-actions github-actions bot marked this pull request as draft May 22, 2025 11:44
Copy link
Contributor

@Pio-Bar Pio-Bar left a comment

Choose a reason for hiding this comment

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

Solid work! My only minor gripe is the use of the overly long selectors. The tests work really well as is but, it makes reading the tests bit confusing.

cy.get('cx-org-unit-details cx-org-card cx-view .main .details .property a[href="/powertools-spa/en/USD/organization/units/Rustic"]' );

Consider replacing with:

cy.get('a').contains(' Rustic ');

Comment on lines 24 to 32
it('initial page load', () => {
cy.get('main cx-view cx-table table');
cy.get('main').a11yRunContinuumTest();
});

it('page loaded', () => {
cy.get('#Rustic');
cy.get('main').a11yRunContinuumTest();
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't running the scan after the page is fully loaded sufficient? I've seen this repeated in the other files too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well in first case its testing skeleton components (where content isn't loaded still) - which in few cases had a11y issues caught by continuum. The second one is validating loaded state (actual table with data).
If this isn't required I can remove the fist one.

Copy link
Contributor

Choose a reason for hiding this comment

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

Testing the skeleton makes sense I would keep it. Let's replace 'initial page load' with sth that makes it clear that the skeleton is the target. At the same time, we need to make sure the skeleton is actually present for the test. Is there a way we could avoid the possible race condition here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed!

cy.get('main').a11yRunContinuumTest();
});

it('account summaries details / initial panel', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should also include a test with the document list populated. The one tested here is empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find any that has documents. They are all empty.

Copy link
Contributor

Choose a reason for hiding this comment

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

What about Custom Retail? I definitely see some 😉
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! Thanks. Applied!

cy.get('main').a11yRunContinuumTest();
});

// it('account summaries details / edit panel', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the commented code be cleaned up?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applied!

cy.get('body').a11yRunContinuumTest();
});

it('unit details / delivery addresses list panel', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Some of these cards the tests navigate to are empty. Wouldn't the test be more complete if we go to populated ones instead?

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed! 👍

@uroslates uroslates marked this pull request as ready for review May 26, 2025 11:22
@uroslates uroslates requested a review from Pio-Bar May 26, 2025 11:22
@github-actions
Copy link
Contributor

Merge Checks Failed

Please push a commit to re-trigger the build.
To push an empty commit you can use `git commit --allow-empty -m "Trigger Build"`

'cx-org-unit-details cx-org-card cx-view .main .details .property a[href="/powertools-spa/en/USD/organization/units/Rustic"]'
);
cy.get('[id="Rustic"]').click();
cy.get('a').contains('Rustic');
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the text is missing spaces (cy.get('a').contains(' Rustic ')). It's currently getting the wrong element and scanning an empty card.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applied!

@github-actions github-actions bot marked this pull request as draft May 27, 2025 08:43
@uroslates uroslates marked this pull request as ready for review May 27, 2025 10:34
@uroslates uroslates requested a review from Pio-Bar May 27, 2025 10:34
@uroslates uroslates marked this pull request as ready for review June 24, 2025 11:31
@github-actions
Copy link
Contributor

Merge Checks Failed

Please push a commit to re-trigger the build.
To push an empty commit you can use `git commit --allow-empty -m "Trigger Build"`

@github-actions github-actions bot marked this pull request as draft June 26, 2025 10:38
@uroslates uroslates marked this pull request as ready for review June 26, 2025 10:43
@github-actions
Copy link
Contributor

Merge Checks Failed

Please push a commit to re-trigger the build.
To push an empty commit you can use `git commit --allow-empty -m "Trigger Build"`

@github-actions github-actions bot marked this pull request as draft June 30, 2025 11:09
@uroslates uroslates marked this pull request as ready for review June 30, 2025 11:26
@uroslates uroslates requested a review from Pio-Bar June 30, 2025 12:03
Pio-Bar
Pio-Bar previously approved these changes Jul 1, 2025
@github-actions github-actions bot marked this pull request as draft July 1, 2025 09:42
@uroslates uroslates marked this pull request as ready for review July 1, 2025 09:55
@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2025

Merge Checks Failed

Please push a commit to re-trigger the build.
To push an empty commit you can use `git commit --allow-empty -m "Trigger Build"`

@github-actions github-actions bot marked this pull request as draft July 1, 2025 11:25
@uroslates uroslates marked this pull request as ready for review July 1, 2025 11:27
@uroslates uroslates requested a review from Pio-Bar July 1, 2025 11:42
@github-actions github-actions bot marked this pull request as draft July 2, 2025 07:53
@uroslates uroslates marked this pull request as ready for review July 2, 2025 07:56
@github-actions github-actions bot marked this pull request as draft July 2, 2025 11:34
@uroslates uroslates marked this pull request as ready for review July 2, 2025 11:38
@uroslates uroslates merged commit 991ddbe into develop Jul 2, 2025
32 checks passed
@uroslates uroslates deleted the feature/CXSPA-10033-b2b-mycompany branch July 2, 2025 12:01
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.

5 participants