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

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