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

Hi Akshay,

Please find the attached rebase patch. Looks like there were some commits after my today's git pull.

Thanks,

Yogesh Mahajan

QA - Team

EnterpriseDB Corporation

Phone: +91-9741705709

On Fri, May 8, 2020 at 2:35 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Hi Yogesh

Unable to apply the patch on the latest code. Please rebase and send it again.

On Fri, May 8, 2020 at 2:05 PM Yogesh Mahajan <yogesh.mahajan@enterprisedb.com> wrote:

Attaching patch 

Thanks,

Yogesh Mahajan

QA - Team

EnterpriseDB Corporation

Phone: +91-9741705709

On Fri, May 8, 2020 at 1:57 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Hi Yogesh

You forgot to attach patch :)

On Fri, May 8, 2020 at 1:35 PM Yogesh Mahajan <yogesh.mahajan@enterprisedb.com> wrote:

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.joshi@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:

• "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.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 <akshay.joshi@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.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 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.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 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.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

--

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

--

Thanks & Regards

Akshay Joshi

Sr. Software Architect

EnterpriseDB Software India Private Limited

Mobile: +91 976-788-8246