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 install prestashop test - Smoke tests #15031

Merged

Conversation

@nesrineabdmouleh
Copy link
Contributor

commented Aug 8, 2019

Questions Answers
Branch? develop
Description? Add a waitFor with timeout to each installation step and add their selectors
Type? refacto
Category? TE
BC breaks? no
Deprecations? no
Fixed ticket? no
How to test? URL_FO=Your_Shop_URL TEST_PATH="smoke/installShop" npm run specific-test

This change is Reviewable

@nesrineabdmouleh nesrineabdmouleh requested a review from PrestaShop/prestashop-core-developers as a code owner Aug 8, 2019
@@ -2,5 +2,5 @@
"slow": 45000,
"timeout": 0,
"reporter": "mochawesome",
"require" : "./campaigns/utils/helpers.js"
"require": "./campaigns/utils/helpers.js"

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Aug 8, 2019

Contributor

👍

await this.page.waitFor(this.installModulesStep, {visible: true});
await this.page.waitFor(this.installModulesAddons, {visible: true, timeout: 360000});
await this.page.waitFor(this.installThemeStep, {visible: true});
await this.page.waitFor(this.installFixturesStep, {visible: true});
await this.page.waitForSelector(this.finalStepPageTitle, {visible: true, timeout: 90000});
await this.checkStepTitle(this.finalStepPageTitle, this.finalStepEnTitle);

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Aug 8, 2019

Contributor

Since you're using the method checkStepTitle only once, maybe it's not really necessary to have it as a separate method ?

This comment has been minimized.

Copy link
@nesrineabdmouleh

nesrineabdmouleh Aug 8, 2019

Author Contributor

@SimonGrn the method checkStepTitle is used many times in installShop.js

This comment has been minimized.

Copy link
@matthieu-rolland

matthieu-rolland Aug 8, 2019

Contributor

Using a function only once is not necessarily a problem, sometimes it's nice to encapsulate a few instructions belonging to the same logic in a function, and give it a descriptive name, so that the reader immediately knows what the coder intended to do when he sees the function call.

But that's just my opinion, not really an official good practice :D

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Aug 8, 2019

Contributor

Damn I got roasted on this one 😆
That's good for me then !

This comment has been minimized.

Copy link
@matthieu-rolland

matthieu-rolland Aug 8, 2019

Contributor

haha, I didn't mean it that way :D

Copy link
Contributor

left a comment

Good to me, thanks !

@matthieu-rolland matthieu-rolland merged commit 46b5b5b into PrestaShop:develop Aug 8, 2019
2 checks passed
2 checks passed
PrettyCI Code formatting
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@matks matks added this to the 1.7.7.0 milestone Sep 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.