Re: [pgAdmin 4 - Housekeeping #5255] Implement Selenium Grid using multi-threading & solenoid using current test framework

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 <http://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 supported
browsers with version set to null so that it will run on the latest browser
version.
- "selenoid_url": "Selenoid Url" should be changed to "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 well.

On Mon, May 4, 2020 at 4:27 PM Yogesh Mahajan <
yogesh(dot)mahajan(at)enterprisedb(dot)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 <akshay(dot)joshi(at)enterprisedb(dot)com>
> 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(dot)mahajan(at)enterprisedb(dot)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(dot)agarwal(at)enterprisedb(dot)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 write
>>>> a logic so that it will drop all the database which starts with name
>>>> ‘acceptance_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 = 60
>>>>  *while *retry > 0:
>>>> try:
>>>> driver.get(url)
>>>> except WebDriverException:
>>>> retry -= 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() function 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(dot)gadakh(at)enterprisedb(dot)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 approached
>>>>> 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(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi Navnath
>>>>>>
>>>>>> Can you please review it?
>>>>>>
>>>>>> On Mon, Apr 13, 2020 at 2:40 PM Yogesh Mahajan <
>>>>>> yogesh(dot)mahajan(at)enterprisedb(dot)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*
>>
>

--
*Thanks & Regards*
*Akshay Joshi*

*Sr. Software Architect*
*EnterpriseDB Software India Private Limited*
*Mobile: +91 976-788-8246*