Hi On Fri, Apr 7, 2017 at 8:01 PM, Navnath Gadakh < navnath.gad...@enterprisedb.com> wrote:
> Hello Joao, > > Thanks for review and suggestions. > > On Fri, Apr 7, 2017 at 9:08 PM, Joao Pedro De Almeida Pereira < > jdealmeidapere...@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". Thanks. -- Dave Page VP, Chief Architect, Tools & Installers EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company Blog: http://pgsnake.blogspot.com Twitter: @pgsnake
