Hi,
Please find the updates patch with above review comments.
Patch adds below functionality to existing framework -
1.Ability to run features in parallel using solenoid(selenium + docker).
a.Selenoid setup steps are included in /regression/README
b.'python runtests.py --pkg feature_tests --parallel' will trigger
parallel feature tests.
2.Removes dependency for pyperclip python module.
3.New script in ../tools/update_selenoid_browsers.py updates browser
images at local selneoid server setup.
Thanks,
Yogesh Mahajan
QA - Team
EnterpriseDB Corporation
Phone: +91-9741705709
On Tue, May 5, 2020 at 3:58 PM Akshay Joshi <akshay.jo...@enterprisedb.com>
wrote:
> 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.maha...@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 <
>> akshay.jo...@enterprisedb.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.maha...@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.agar...@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
>>>>> 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.gad...@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
>>>>>> 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.jo...@enterprisedb.com> wrote:
>>>>>>
>>>>>>> Hi Navnath
>>>>>>>
>>>>>>> Can you please review it?
>>>>>>>
>>>>>>> On Mon, Apr 13, 2020 at 2:40 PM Yogesh Mahajan <
>>>>>>> yogesh.maha...@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*
>>>
>>
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
>
> *Sr. Software Architect*
> *EnterpriseDB Software India Private Limited*
> *Mobile: +91 976-788-8246*
>
