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

Fix broken selectors and logic for the test suite 9 (customer) #13570

Merged
merged 9 commits into from Apr 29, 2019

Conversation

@SimonGrn
Copy link
Contributor

SimonGrn commented Apr 25, 2019

Questions Answers
Branch? 1.7.6.x
Description? Fix for broken selectors and logic for the test suite 9 (customer)
Type? refacto
Category? TE
BC breaks? no
Deprecations? no
Fixed ticket?
How to test? TEST_PATH=full/09_customer/customer_settings/* npm run specific-test -- --URL=URL_PRESTASHOP

This change is Reviewable

@SimonGrn SimonGrn requested a review from PrestaShop/prestashop-core-developers as a code owner Apr 25, 2019
@prestonBot

This comment has been minimized.

Copy link
Collaborator

prestonBot commented Apr 25, 2019

Hello @SimonGrn!

This is your first pull request on the PrestaShop project. Thank you, and welcome to this Open Source community!

@SimonGrn SimonGrn added the TE label Apr 25, 2019
@SimonGrn SimonGrn added this to In progress in PrestaShop 1.7.6 via automation Apr 25, 2019
@SimonGrn SimonGrn added this to the 1.7.6.0 milestone Apr 25, 2019
fix for tests 1 and 2 of the shop_parameters test suite
tests/E2E/test/campaigns/common_scenarios/customer.js Outdated Show resolved Hide resolved
select_customer: '//table[@id="customer_grid_table"]//td[contains(@class, "bulk_action-type")]/div/label',
bulk_actions_button: '//button[contains(@class, "js-bulk-actions-btn")]',
bulk_actions_delete_button: '//button[@id="customer_grid_bulk_action_delete_selection"]',
empty_list_icon: '//*[@id="customer_grid_table"]//tbody//td/div/p[2]',

This comment has been minimized.

Copy link
@Quetzacoalt91

Quetzacoalt91 Apr 25, 2019

Member

How can we make this selector cleaner?

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Apr 25, 2019

Author Contributor

This element is not easily targetable: no ID, no special class or anything that would make the selector cleaner. I could be able to transform it into a CSS one: it might look cleaner but that won't change the fact that it's kinda hard to target.

This comment has been minimized.

Copy link
@Quetzacoalt91

Quetzacoalt91 Apr 25, 2019

Member

I understand that, my question was more "can we (the core team) do something to make this selector cleaner?"
If you could show screenshot of the element you want to target, we may add an attribute on it.

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Apr 25, 2019

Author Contributor

image
Oh my bad I didn't understand. That would be really cool !
This selector targets the checkbox in front of every customer, that you use when you want to perform bulk actions.

This comment has been minimized.

Copy link
@Quetzacoalt91

Quetzacoalt91 Apr 25, 2019

Member

Hmm, for this one it looks like you can rely on the class js-bulk-action-checkbox, doesn't it?

In this part of code, I was talking about the line 27

empty_list_icon: '//*[@id="customer_grid_table"]//tbody//td/div/p[2]',

which seems really tricky.

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Apr 26, 2019

Author Contributor

I can't target the checkbox itself because it's not visible and Selenium won't let me interact with it, I have to target the whole visible element which is the "label" tag.
For the second one, there was an ID on the image before. I could remove some of the selectors in the second part (like "td/div") but since I'm looking for the "No records found" icon I know there's only one td and one div in the table.
This is what the markups looks like (and I'm only interested in the "p" tag):
no_records_found
I thought about using the "mb-2" class but IMHO it's too generic and I can't be sure there won't be another one in the page or even in the table.

This comment has been minimized.

Copy link
@Quetzacoalt91

Quetzacoalt91 Apr 26, 2019

Member

Can you have a look at 0ba6582 included in this PR ? I added a class on the parent div to help you target the empty state.

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Apr 26, 2019

Author Contributor

That's perfect.

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Apr 29, 2019

Author Contributor

@Quetzacoalt91 I modified the selector to use your new class. Test is OK.
Thanks again ! 👌

@SimonGrn SimonGrn changed the title Customer Fix broken selectors and logic for the test suite 9 (customer) Apr 25, 2019
@colinegin colinegin moved this from In progress to To be reviewed in PrestaShop 1.7.6 Apr 26, 2019
@Quetzacoalt91 Quetzacoalt91 merged commit 254461c into PrestaShop:1.7.6.x Apr 29, 2019
2 checks passed
2 checks passed
PrettyCI Code formatting
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
PrestaShop 1.7.6 automation moved this from To be reviewed to Done Apr 29, 2019
@Quetzacoalt91

This comment has been minimized.

Copy link
Member

Quetzacoalt91 commented Apr 29, 2019

Scenario tested. Thank you @SimonGrn

@SimonGrn SimonGrn deleted the SimonGrn:customer branch Oct 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
3 participants
You can’t perform that action at this time.