Re: [pgadmin-hackers] pgAdmin4: Test result enhancement patch - Mailing list pgadmin-hackers

From Navnath Gadakh
Subject Re: [pgadmin-hackers] pgAdmin4: Test result enhancement patch
Date
Msg-id CAOAJCYok7-PmPVCW_H1jgeU6brPmFmGw_CyQsi+Ot95FYdW0GA@mail.gmail.com
Whole thread Raw
In response to Re: [pgadmin-hackers] pgAdmin4: Test result enhancement patch  (Dave Page <dave.page@enterprisedb.com>)
Responses Re: [pgadmin-hackers] pgAdmin4: Test result enhancement patch
List pgadmin-hackers
Hi Dave,

      Please find the revised patch.

On Mon, Apr 10, 2017 at 1:43 PM, Dave Page <dave.page@enterprisedb.com> wrote:
Hi

On Fri, Apr 7, 2017 at 8:01 PM, Navnath Gadakh <navnath.gadakh@enterprisedb.com> wrote:
Hello Joao,

          Thanks for review and suggestions.

On Fri, Apr 7, 2017 at 9:08 PM, Joao Pedro De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
Hello Hackers,
We just looked into this patch and have some questions about it.

- Where is the JSON file created? Do we need to call it with some special argument in order for it to be created?
 This file is created under /regression/ directory (runtests.py, line 401). No need to call any special argument.
- There is a print statement in the line 275 of runtests.py is it supposed to be there?
Removed. 
- The function name addSuccess does not match the styling of the code, it should be add_success
Done. 
Suggestions:
- The definition of the class_name (run_tests.py, line 229) variable looks the same as in the if statements below and could be extracted into a function to avoid repeating the same code.
Done. 
- In the same function when we are updating error/failure/skip test results the code looks pretty similar and can also be extracted into a function
Done.
 

@Dave, Please find the attached patch with necessary code changes. I have also added missing scenario names to some test cases.

The passed test results are shown as null. They should either be removed (because they'll all be "Passed" or similar anyway), or set to "Passed" or "Pass".
Ok. For consistency purposed with other test case type I have set it to ''Passed".

Also, did some code cleanup in the attached patch.

Thanks.

Thanks.

--
Dave Page
VP, Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake



--
Regards,
Navnath Gadakh

EnterpriseDB Corporation
The Enterprise PostgreSQL Company


Attachment

pgadmin-hackers by date:

Previous
From: Murtuza Zabuawala
Date:
Subject: Re: [pgadmin-hackers] [pgAdmin4][PATCH] To fix the issue in browser tree
Next
From: Dave Page
Date:
Subject: [pgadmin-hackers] pgAdmin 4 commit: Bump version numbers for release.