Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jVuoP-0000GV-P4 for pgadmin-hackers@arkaria.postgresql.org; Tue, 05 May 2020 10:28:30 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1jVuoO-00043t-0n for pgadmin-hackers@arkaria.postgresql.org; Tue, 05 May 2020 10:28:28 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jVuoN-0003tT-Hi for pgadmin-hackers@lists.postgresql.org; Tue, 05 May 2020 10:28:27 +0000 Received: from mail-il1-x132.google.com ([2607:f8b0:4864:20::132]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1jVuoJ-0003T1-Py for pgadmin-hackers@postgresql.org; Tue, 05 May 2020 10:28:26 +0000 Received: by mail-il1-x132.google.com with SMTP id c18so1764483ile.5 for ; Tue, 05 May 2020 03:28:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=cAnrfeCHX/qTg4HqGrR+PdtY64MMQABDX/o3k3IvxPE=; b=hPxuALQReSQ96ABVut8mVAlUvVoI3vFRRPJBYMWarynMNviVVxvqrDwIOn6UUBHNiw 0Wg20mTmyG0U1G9F9gnjWcsMfdZNuuzQehaC5Husq4ePUF/cVThT9oq05aARelapxay4 VXZlXPFHOd7brgx4TGyHetKem6jicuginhWqUpSJJrJwILcRNDLp5Jm+SJwPXfgAwEs0 Col7ymiRliRe65PZGQYwp0q86ywJOitutdD/ra6Uvt6Y6j5vdPZ5Z/jl4LNcBNbrIyma H4mtFfv+P94lA24d1nXsO8biZ433qYPrgZ2KbGK5hGpbXNCt2hf9/5smzt4yO88IhBSd Jl1w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=cAnrfeCHX/qTg4HqGrR+PdtY64MMQABDX/o3k3IvxPE=; b=adXfkldzchlulx7k+jktvrGCZDzCL84WfLVhFsJ+n0ljCjFixYMqwYP/ZIbWWiFRt/ OlNpXLHQ1/NjYn67P/2Vm7jtoixnCwQ12O/UClj1MufkP8YDsmia6pG5qzJp/T5k0Pho pXwjYlCJBpf+RxIRTIMS1g6mE8x/Y9H/r1YY6OHW6fdxRhH83znywEdlk3wSWr+3DUIp S8qeDv+xdp4kQmRK0+X2j/1CvDxMQHtIrdfZxC0KHWl5ZbPqRAS3bGitJJgDcqproT/5 oUAj2pK/GDsxgFn+EnCXqElPyZG2Cjvg2yOjdTdI9B/KrTULrZq4LpzHshg/4QShS0KJ F8qg== X-Gm-Message-State: AGi0PubHTPYN0BzyOL98/nYlomGCj7+GfESETfDnoLD3kc/Uh8oJt5nJ ee5oZ46rlURZHf/Hupv118nzlPYedLzWrLKYtfHV3N2j0flu3Xp26YjQ/4Hp0GhoJCh4S34pKYw uRmNW8ZtygKGvjwZxgY3wKZ1SyKIQof5ugWR6yx88E28NH9oUsYFPE/Ry9OFcioaDovdn9zonkB MPMAAtW0QOm7U34hASHii3L68jIiJe82S2qXeX35yQ0z62lg73aNvZbmg+ow== X-Google-Smtp-Source: APiQypLerOzlz3ifqeEaBlCjl6xO+t/G3Edl7+lFcVzKaaZGKHH2eFH5cjagt7oM6bKyZzHhy6PHFVLD62L1D/fLzRs= X-Received: by 2002:a92:49c7:: with SMTP id k68mr2789334ilg.252.1588674501958; Tue, 05 May 2020 03:28:21 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Akshay Joshi Date: Tue, 5 May 2020 15:58:10 +0530 Message-ID: Subject: Re: [pgAdmin 4 - Housekeeping #5255] Implement Selenium Grid using multi-threading & solenoid using current test framework To: Yogesh Mahajan Cc: pgadmin-hackers , Shubham Agarwal , navnath gadakh Content-Type: multipart/alternative; boundary="0000000000005a784f05a4e41b06" X-CLOUD-SEC-AV-Info: enterprisedb,google_mail,monitor X-CLOUD-SEC-AV-Sent: true X-Gm-Spam: 0 X-Gm-Phishy: 0 List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --0000000000005a784f05a4e41b06 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Yogesh Following are the review comments: - *pyjq* package is not required as we used it only in one place. A result is a normal dictionary that can be easily looped through. - Remove "*if (SUPPORT_SSH_TUNNEL is True and ...*" logic from config.py, we have already removed that. - Remove yarn.lock file. - Remove *pyperclip *from the regression/requirements.txt as we are not using it. - Please mentioned the value of *pgAdmin_default_server *should not be ' *127.0.0.1*' in the README file even though everything runs on the same machine. - Please mentioned that if we set the value of the browser version is *null* then selenoid will take the latest available browser version. - Got the below error if selenoid_url is not provided: - list index out of range Unable to find Selenoid Status *test_config.json.in *: - "selenoid_info" should be renamed to "selenoid_config". Proper alignment is required. - "cross_Browsers" should be renamed to "cross_browsers" or "run_on_browsers" or "run_tests_on_browsers". Provide entries for suppor= ted browsers with version set to null so that it will run on the latest brow= ser version. - "selenoid_url": "Selenoid Url" should be changed to "selenoid_url": "http://:4444/wd/hub". If you change the names in test_config.json.in then please update the same in README as well. On Mon, May 4, 2020 at 4:27 PM Yogesh Mahajan < yogesh.mahajan@enterprisedb.com> wrote: > Hi Akshay, > > Please find the updated patch. > > Thanks, > Yogesh Mahajan > QA - Team > EnterpriseDB Corporation > > Phone: +91-9741705709 > > > On Mon, May 4, 2020 at 2:51 PM Akshay Joshi > wrote: > >> Hi Yogesh >> >> The patch is not applied to the master branch. Can you please rebase and >> send the patch again. >> >> On Fri, May 1, 2020 at 12:28 PM Yogesh Mahajan < >> yogesh.mahajan@enterprisedb.com> wrote: >> >>> Hi, >>> >>> Please find updated patch modified according to review comments - >>> Patch implements below things - >>> 1.Enable the current framework to provide option to execute Feature >>> tests in parallel on selenium grid set up. >>> - Addition of new switch to start parallel features tests. >>> - New parameters with respect to selenoid in test_config.json.in >>> - Addition of new script to check solenoid updates. >>> >>> >>> >>> Thanks, >>> Yogesh Mahajan >>> QA - Team >>> EnterpriseDB Corporation >>> >>> Phone: +91-9741705709 >>> >>> >>> On Tue, Apr 21, 2020 at 1:18 PM Shubham Agarwal < >>> shubham.agarwal@enterprisedb.com> wrote: >>> >>>> Hi Yogesh, >>>> Below are the review comments- >>>> >>>> 1. runtests.py >>>> a. The exception traceback logic at line number 653 in runtests.py >>>> is not correct since it is particular to the thread >>>> but there is much more code in that block which can throw some >>>> exception. >>>> b. line number 447 -> The drop_database function will only try to drop >>>> the database with the name which is newly created >>>> at 431 line number, its probability is 1% instead of this you can writ= e >>>> a logic so that it will drop all the database which starts with name >>>> =E2=80=98acceptance_test_db'. >>>> c. line 584 - Why we are including resql test case execution in GUI >>>> execution logic. >>>> d. Change the function name run_test as script name is also >>>> runtests.py >>>> >>>> 2. test_utils.py >>>> a. Remove the headless chrome code from get_remote_webdriver() in >>>> test_utils.py since we are using solenoid and it is not required >>>> anymore. >>>> b. Create separate functions to instantiate the firefox driver and >>>> chrome driver logic since the same code is used in multiple files. >>>> c. launch_url_in_browser() -> you can simplify the definition of the >>>> function like: >>>> retry =3D 60 >>>> =EF=BB=BF *while *retry > 0: >>>> try: >>>> driver.get(url) >>>> except WebDriverException: >>>> retry -=3D 1 >>>> 3. Execution logs are not printing as per the logic some time, I ran >>>> the suite for two servers and attached are the execution logs. >>>> 4. Readme - >>>> Please provide the Valid selenoid URL to be provided in the >>>> test_config.json, with all the steps mentioned in the readme it is not >>>> clear. >>>> Revisit the readme and write the missing steps. >>>> 5. copy_selected_query_results_feature_tests.py- >>>> Create the function to avoid duplicate code. The code for pasting the >>>> values is repeating 8 times in the test code. >>>> 6. Provide the valid docstring in newly introduced functions and also >>>> valid comments while calling it. for ex.- _update_preference() functio= n is >>>> introduced in pg_utilities_backup_restrore_test.py but from the >>>> function name, it is not clear what preferences are going to update in= it. >>>> 7. test_index_constraint_add test case is failing due to the latest >>>> change, please merge and update this test case >>>> >>>> On Thu, Apr 16, 2020 at 2:41 PM navnath gadakh < >>>> navnath.gadakh@enterprisedb.com> wrote: >>>> >>>>> Hi, >>>>> I think I am not the right person to review this patch now as I >>>>> already reviewed this code offline in the last week. I know the appro= ached >>>>> Yogesh has followed, also given some review comments on it. >>>>> Someone else please review it. >>>>> >>>>> Thanks! >>>>> >>>>> On Mon, Apr 13, 2020 at 2:49 PM Akshay Joshi < >>>>> akshay.joshi@enterprisedb.com> wrote: >>>>> >>>>>> Hi Navnath >>>>>> >>>>>> Can you please review it? >>>>>> >>>>>> On Mon, Apr 13, 2020 at 2:40 PM Yogesh Mahajan < >>>>>> yogesh.mahajan@enterprisedb.com> wrote: >>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> Please find the attached patch for running *features tests* using >>>>>>> solenoid(selenium grid + docker). >>>>>>> KIndly review. >>>>>>> To sun feature tests in parallel, required prerequisites can be >>>>>>> checked in '~/web/regression/README' file. >>>>>>> Also detailed instructions are added in the same file. >>>>>>> After applying the patch, any existing process for execution of >>>>>>> API/Features tests remains the same. >>>>>>> >>>>>>> >>>>>>> Thanks, >>>>>>> Yogesh Mahajan >>>>>>> QA - Team >>>>>>> EnterpriseDB Corporation >>>>>>> >>>>>>> Phone: +91-9741705709 >>>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> *Thanks & Regards* >>>>>> *Akshay Joshi* >>>>>> >>>>>> *Sr. Software Architect* >>>>>> *EnterpriseDB Software India Private Limited* >>>>>> *Mobile: +91 976-788-8246* >>>>>> >>>>> >>>>> >>>>> -- >>>>> Regards, >>>>> Navnath Gadakh >>>>> >>>> >>>> >>>> -- >>>> Thanks & Regards, >>>> Shubham Agarwal >>>> EnterpriseDB Corporation >>>> >>>> The Postgres Database Company >>>> >>> >> >> -- >> *Thanks & Regards* >> *Akshay Joshi* >> >> *Sr. Software Architect* >> *EnterpriseDB Software India Private Limited* >> *Mobile: +91 976-788-8246* >> > --=20 *Thanks & Regards* *Akshay Joshi* *Sr. Software Architect* *EnterpriseDB Software India Private Limited* *Mobile: +91 976-788-8246* --0000000000005a784f05a4e41b06 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi=C2=A0Yogesh

Following are the review= comments:
  • pyjq package is not required as we= used it only in one place. A result is a normal dictionary that can be eas= ily looped=C2=A0through.
  • Remove "if (SUPPORT_SSH_TUNNEL is = True and ..." logic from config.py, we have already removed that.<= /li>
  • Remove yarn.lock file.
  • Remove=C2=A0pyperclip from th= e regression/requirements.txt as we are not using it.
  • Please mentio= ned the value of=C2=A0pgAdmin_default_server should not be '1= 27.0.0.1' in the README file even though everything runs on the sam= e machine.
  • Please mentioned that if we set the value of the browser= version is null then selenoid=C2=A0will take the latest available b= rowser version.
  • Got the below error if selenoid_url is not provided= :
    • list index out of range
      Unable to find Selenoid Status
  • "selenoid_info" should be ren= amed to "selenoid_config". Proper alignment is required.
  • = "cross_Browsers" should be renamed to "cross_browsers" = or "run_on_browsers" or "run_tests_on_browsers". Provid= e entries for supported browsers with version set to null so that it will r= un on the latest browser version.
  • "selenoid_url": "S= elenoid Url" should be changed to=C2=A0"selenoid_url": "= ;http://<IP address of Selenoid Installed machine>:4444/wd/hub".=
If you change the names in test_config.json.in then please update the same in README as wel= l.

On Mon, May 4, 2020 at 4:27 PM Yogesh Mahajan &l= t;yogesh.mahajan@enterpr= isedb.com> wrote:
Hi Akshay,

Please find the u= pdated patch.

Tha= nks,
Yogesh Mahajan
QA - Team
EnterpriseDB Cor= poration

Phone: +91-9741705709


On Mon, May 4, 2020 at 2:51 PM Akshay Joshi <akshay.joshi@enterprisedb.c= om> wrote:
Hi=C2=A0Yogesh

The patch is not appli= ed to the master branch. Can you please rebase and send the patch again.

On Fri, May 1, 2020 at 12:28 PM Yogesh Mahajan <yogesh.mahajan@enterprisedb= .com> wrote:
Hi,

Please find updated patch modif= ied according to review comments=C2=A0-=C2=A0
Patch implements be= low things=C2=A0-=C2=A0
1.Enable the current framework to provide= option to execute Feature tests in parallel =C2=A0on selenium grid set up.=
=C2=A0 =C2=A0- Addition of new switch to start parallel features= tests.
=C2=A0 =C2=A0- New parameters with respect to selenoid in= test_config.json.= in
=C2=A0 =C2=A0- Addition of new script to check solenoid=C2= =A0updates.



=
Thanks,
Yogesh Mahajan
QA -= Team
EnterpriseDB Corporation

Phone: +91-9741705709<= /div>


On Tue, Apr 21, 2020 at 1:18 PM Shub= ham Agarwal <shubham.agarwal@enterprisedb.com> wrote:
Hi Yogesh,
Below are the review comments-

1. runtests.py
=C2=A0 = =C2=A0 a. The excepti= on traceback logic at line number 653 in runtests.py is not correct since i= t is particular to the thread
= but there is much more code in that block which can throw some exception.
b. line number 447 -> The drop_database functio= n will only try to drop the database with the name which is newly created <= /span>
at 431 line number, its probabi= lity is 1% instead of this you can write a logic so that it will drop all t= he database which starts with name =E2=80=98acceptance_test_db'.=
= =C2=A0 c. line 584 -= =C2=A0Why we are including resql test case execution in GUI execution logic= .
=C2=A0 =C2=A0 d.=C2=A0<= span style=3D"color:rgb(0,0,0);font-size:9pt">Change the function name run_= test as script name is also runtests.py

2. test_utils.py
=C2= =A0 =C2=A0 a.=C2=A0Remove the headless c= hrome code from get_remote_webdriver() in test_utils.py since we are using = solenoid and it is not required anymore.
=C2=A0 =C2=A0 b. Create separate functions to instantiate the firefox driver and c= hrome driver logic since the same code is used in multiple files.
c. launch_url_in_browser() -> you can simplify the definition = of the function like:
=C2=A0 =C2=A0=C2=A0retry =3D 60
=EF=BB=BF=C2=A0 =C2=A0 =C2=A0 =C2=A0 while=C2=A0retry > 0:
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 try:
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 driver.get(url)
=C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 except WebDriverException:
=C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0retry -=3D 1
3. Execution logs are not printing as per the logic some time, I = ran the suite for two servers and attached are the execution logs.
4. Readme -
<= div> Please provide the Valid selenoid URL to = be provided in the test_config.json, with all the steps mentioned in the re= adme it is not clear.
= Revisit the readme and write the missing steps.
5. copy_selected_query_results_feature_tests.py-
Create the function to= avoid duplicate code. The code for pasting the values is repeating 8 times= in the test code.
6. Pr= ovide the valid docstring in newly introduced functions and also valid comm= ents while calling it. for ex.- _update_preference() function is
introduced in pg_utilities_back= up_restrore_test.py but from the function name, it is not clear what prefer= ences are going to update in it.
7. test_in= dex_constraint_add test case is failing due to the latest change, please me= rge and update this test case

<= /font>
On Thu, Apr 16, 2020 at 2:41 PM navnath gadakh= <n= avnath.gadakh@enterprisedb.com> wrote:
Hi,
I th= ink I am not the right person to review this patch now as I already reviewe= d this code offline in the last week. I know the=C2=A0approached Yogesh has= followed, also given some review comments on it.
Someone else please review it.=C2=A0
<= div>
Thanks!

On Mon, Apr 13, 2020 at 2:49 PM Akshay J= oshi <akshay.joshi@enterprisedb.com> wrote:
Hi Navnath

Can you please review it?

On Mon, Apr 13, 2020 at 2:40 PM Yogesh Mahajan <yogesh.mahajan@enterpr= isedb.com> wrote:
Hi,

Please find the attached=C2=A0patch for= running features tests using solenoid(selenium=C2=A0grid + docker).=
KIndly review.
To sun feature tests in parallel, = required prerequisites can be checked in '~/web/regression/README' = file.
Also detailed=C2=A0= instructions are added in the same=C2=A0file.
After applying the patch, any existing process for e= xecution of API/Features=C2=A0tests remains the same.
=

Thanks,
Yogesh Mahajan
Enterpri= seDB Corporation

Phone: +91-9741705709


--
Thanks & Regards=
A= kshay Joshi
Sr. Software Architect
EnterpriseDB Software India Private Limited
Mobile: +91 976-788-8246


--
Regards,
Navnath = Gadakh


--
Thanks & Regards,
Shubham Agarwal
EnterpriseDB Corporation

The Postgres Database Company


--
Thanks & Regards
Akshay Joshi
<= font color=3D"#3333FF">Sr. Software Architect=
<= font color=3D"#000000" face=3D"arial, sans-serif">EnterpriseDB Software = India Private Limited
Mobile: +91 976-788-8246=


--
Thanks & Regards
=
Akshay Joshi
Sr. Software Architect
EnterpriseDB Software India Private Limited
=
Mobile: += 91 976-788-8246
--0000000000005a784f05a4e41b06--