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

Functional Tests - Add BO tests for customers #15865

Merged
merged 20 commits into from Oct 15, 2019

Conversation

@boubkerbribri
Copy link
Contributor

boubkerbribri commented Oct 8, 2019

Questions Answers
Branch? develop
Description? Adding Functional tests for BO customers
Type? refacto
Category? TE
BC breaks? no
Deprecations? no
Fixed ticket? no
How to test? TEST_PATH="functional/BO/customers/customers/*" URL_FO=SHOP_URL npm run specific-test

This change is Reviewable

@boubkerbribri boubkerbribri requested a review from PrestaShop/prestashop-core-developers as a code owner Oct 8, 2019
@boubkerbribri boubkerbribri added the WIP label Oct 8, 2019
@boubkerbribri boubkerbribri added this to the 1.7.7.0 milestone Oct 8, 2019
@boubkerbribri boubkerbribri added TE and removed WIP labels Oct 8, 2019
Copy link
Contributor

PierreRambaud left a comment

Reviewed 8 of 12 files at r1, 1 of 1 files at r3, 2 of 2 files at r4, 1 of 1 files at r5.
Reviewable status: all files reviewed, 14 unresolved discussions (waiting on @boubkerbribri, @nesrineabdmouleh, and @SimonGrn)


tests/puppeteer/campaigns/data/demo/customer.js, line 9 at r5 (raw file):

    email: 'pub@prestashop.com',
    password: '123456789',
    enabled: 'Yes',

Any possibilities to use boolean with a convertor if the string is needed?


tests/puppeteer/campaigns/data/faker/customer.js, line 17 at r5 (raw file):

    this.monthOfBirth = customerToCreate.monthOfBirth || (this.birthDate.getMonth() + 1).toString();
    this.dayOfBirth = customerToCreate.dayOfBirth || this.birthDate.getDate().toString();
    this.enabled = customerToCreate.enabled || 'Yes';

Same here, I think this should be a boolean.


tests/puppeteer/campaigns/functional/BO/customers/customers/01_filter&QuickEditCustomers.js, line 1 at r5 (raw file):

// Using chai

you must rename your file, having special char like & is a very bad idea since it's a Linux instruction.


tests/puppeteer/campaigns/functional/BO/customers/customers/01_filter&QuickEditCustomers.js, line 40 at r5 (raw file):

  loginCommon.loginBO();
  it('should go to Customers page', async function () {
    await this.pageObjects.boBasePage.goToSubMenu(
    await this.pageObjects.boBasePage.goToSubMenu(
      this.pageObjects.boBasePage.customersParentLink,
      this.pageObjects.boBasePage.customersLink
    );

I was searching the end of the function's call ^^'


tests/puppeteer/campaigns/functional/BO/customers/customers/01_filter&QuickEditCustomers.js, line 51 at r5 (raw file):

    }
    numberOfCustomers = await this.pageObjects.customersPage.getNumberFromText(
      this.pageObjects.customersPage.customerGridTitle);

same here, always prefer having the begin and the end of the declaration at the same line.


tests/puppeteer/campaigns/functional/BO/customers/customers/01_filter&QuickEditCustomers.js, line 246 at r5 (raw file):

  });

  describe('Quick Edit Customers', async () => {

You should separaeter describe in separated files, or add a comment for better readability.
Everything is packed in this file, adding lines break could help :)


tests/puppeteer/campaigns/functional/BO/customers/customers/01_filter&QuickEditCustomers.js, line 262 at r5 (raw file):

        '1',
        'active',
        'No');

Same here too (and everywhere)


tests/puppeteer/campaigns/functional/BO/customers/customers/02_CRUDCustomerInBO.js, line 54 at r5 (raw file):

    await this.pageObjects.boBasePage.goToSubMenu(
      this.pageObjects.boBasePage.customersParentLink,
      this.pageObjects.boBasePage.customersLink);

same as previous


tests/puppeteer/campaigns/functional/BO/customers/customers/02_CRUDCustomerInBO.js, line 93 at r5 (raw file):

    });
  });
  describe('View Customer Created', async () => {

For better readability you maybe need to separate describe in files.
Instead of having all CRUD in one file, create four files :)


tests/puppeteer/campaigns/functional/BO/customers/customers/03_customersBulkActionsInBO.js, line 1 at r5 (raw file):

// Using chai

What is the reason to have number in file name?


tests/puppeteer/pages/BO/addCustomer.js, line 35 at r5 (raw file):

   * @return {Promise<textContent>}
   */
  async createEditCustomer(customerData) {

No export?

Copy link
Contributor Author

boubkerbribri left a comment

Reviewable status: 5 of 12 files reviewed, 14 unresolved discussions (waiting on @nesrineabdmouleh, @PierreRambaud, and @SimonGrn)


tests/puppeteer/campaigns/data/demo/customer.js, line 9 at r5 (raw file):

Previously, PierreRambaud (GoT) wrote…

Any possibilities to use boolean with a convertor if the string is needed?

Done. Thanks


tests/puppeteer/campaigns/data/faker/customer.js, line 17 at r5 (raw file):

Previously, PierreRambaud (GoT) wrote…

Same here, I think this should be a boolean.

Done.


tests/puppeteer/campaigns/functional/BO/customers/customers/01_filter&QuickEditCustomers.js, line 3 at r3 (raw file):

Previously, PierreRambaud (GoT) wrote…

I recommend you to use https://www.npmjs.com/package/module-alias instead which is more maintained and use by a lot of good js libraries :)

will be added in other PR


tests/puppeteer/campaigns/functional/BO/customers/customers/01_filter&QuickEditCustomers.js, line 1 at r5 (raw file):

Previously, PierreRambaud (GoT) wrote…

you must rename your file, having special char like & is a very bad idea since it's a Linux instruction.

Done.


tests/puppeteer/campaigns/functional/BO/customers/customers/01_filter&QuickEditCustomers.js, line 40 at r5 (raw file):

Previously, PierreRambaud (GoT) wrote…
    await this.pageObjects.boBasePage.goToSubMenu(
      this.pageObjects.boBasePage.customersParentLink,
      this.pageObjects.boBasePage.customersLink
    );

I was searching the end of the function's call ^^'

Done.


tests/puppeteer/campaigns/functional/BO/customers/customers/01_filter&QuickEditCustomers.js, line 51 at r5 (raw file):

Previously, PierreRambaud (GoT) wrote…

same here, always prefer having the begin and the end of the declaration at the same line.

Done.


tests/puppeteer/campaigns/functional/BO/customers/customers/01_filter&QuickEditCustomers.js, line 246 at r5 (raw file):

Previously, PierreRambaud (GoT) wrote…

You should separaeter describe in separated files, or add a comment for better readability.
Everything is packed in this file, adding lines break could help :)

Done.


tests/puppeteer/campaigns/functional/BO/customers/customers/01_filter&QuickEditCustomers.js, line 262 at r5 (raw file):

Previously, PierreRambaud (GoT) wrote…

Same here too (and everywhere)

Done.


tests/puppeteer/campaigns/functional/BO/customers/customers/02_CRUDCustomerInBO.js, line 54 at r5 (raw file):

Previously, PierreRambaud (GoT) wrote…

same as previous

Done.


tests/puppeteer/campaigns/functional/BO/customers/customers/02_CRUDCustomerInBO.js, line 93 at r5 (raw file):

Previously, PierreRambaud (GoT) wrote…

For better readability you maybe need to separate describe in files.
Instead of having all CRUD in one file, create four files :)

I will discuss it the QA team and do it after (maybe), but for now, we like to have each file (BIG scenario) to be independent and readable at the same file.


tests/puppeteer/campaigns/functional/BO/customers/customers/03_customersBulkActionsInBO.js, line 1 at r5 (raw file):

Previously, PierreRambaud (GoT) wrote…

What is the reason to have number in file name?

It helps us to recognize the tests, and to make the same ranking as tests in the 'Test Manager'.


tests/puppeteer/pages/BO/addCustomer.js, line 35 at r5 (raw file):

Previously, PierreRambaud (GoT) wrote…

No export?

export of what ?

Copy link
Contributor

PierreRambaud left a comment

Reviewed 1 of 7 files at r6.
Reviewable status: 6 of 12 files reviewed, 3 unresolved discussions (waiting on @boubkerbribri, @nesrineabdmouleh, @PierreRambaud, and @SimonGrn)


tests/puppeteer/pages/BO/addCustomer.js, line 35 at r5 (raw file):

Previously, boubkerbribri wrote…

export of what ?

My bad, didn't see it was in a function 😅

Copy link
Contributor

PierreRambaud left a comment

Reviewed 3 of 7 files at r6, 3 of 3 files at r7.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @nesrineabdmouleh and @SimonGrn)

Copy link
Contributor

PierreRambaud left a comment

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @nesrineabdmouleh and @SimonGrn)

@SimonGrn SimonGrn merged commit 16d4ada into PrestaShop:develop Oct 15, 2019
2 of 3 checks passed
2 of 3 checks passed
code-review/reviewable 2 discussions left (nesrineabdmouleh, SimonGrn)
Details
PrettyCI Code formatting
Details
Travis CI - Pull Request Build Passed
Details
@boubkerbribri boubkerbribri deleted the boubkerbribri:functionalCustomers branch Oct 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
PrestaShop 1.7.7
  
Awaiting triage
5 participants
You can’t perform that action at this time.