Re: [pgAdmin4][Patch]: Feature #4202 Implement new framework to test Reverse Engineering SQL

On Mon, Jun 17, 2019 at 8:19 AM Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
wrote:

>
> On Mon, Jun 17, 2019 at 11:54 AM Akshay Joshi <
> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>
>> Hi Dave/Hackers
>>
>> On Fri, Jun 14, 2019 at 6:10 PM Akshay Joshi <
>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>
>>>
>>>
>>> On Fri, Jun 14, 2019 at 1:59 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>
>>>> Hi
>>>>
>>>> On Thu, Jun 13, 2019 at 12:52 PM Akshay Joshi <
>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi Hackers
>>>>>
>>>>> I have implemented the new test framework to test the Reverse
>>>>> Engineering SQL. I have integrated it as a part of API/Regression test
>>>>> suite. It will work when we run all the test cases or module wise test case.
>>>>>
>>>>> *How it works*: Attached patch contains the generic framework to read
>>>>> all the JSON files from the *tests->version based (example 9.6_plus,
>>>>> 10_plus or default) folder. *Run all the test scenarios present in
>>>>> the JSON file in sequential order.
>>>>>
>>>>> Format of the JSON file is mentioned in
>>>>> "<path_of_source>web/pgadmin/browser/server_groups/servers/databases/casts/tests/default/test.json"
>>>>>
>>>>> For expected SQL we will have following two options:
>>>>>
>>>>> - Provide the expected sql in scenario itself as parameter *"expected_sql"
>>>>> : "<SQL>"*.
>>>>> - Create a output file with any name in the same directory where
>>>>> the JSON file resides and specify the parameter "*expected_sql_file":
>>>>> "<name of the file>"*
>>>>>
>>>>> Attached patch contains both the above mentioned examples.
>>>>>
>>>>> Please review it.
>>>>>
>>>>
>>>> Nice!
>>>>
>>>> A few comments:
>>>>
>>>> - The scenario name should be "Reverse Engineered SQL Test Cases"
>>>> - After the scenario name is output, can we output a \n so the next
>>>> line isn't appended to the name?
>>>>
>>>
>>> Will fix the above.
>>>
>>>> - How do we run only the re_sql tests? I tried the obvious ways
>>>> (e.g. python runtests.py --pkg
>>>> regression.re_sql.tests.test_resql.ReverseEngineeringSQLTestCase) but got
>>>> errors. Please add an example to web/regression/README.
>>>>
>>>
>>> It is not a pgadmin module and we have kept it in regression folder,
>>> so will have to change the existing code. I have tried but facing issues
>>> when run only "regression.re_sql.tests", will continue working on this.
>>>
>>
>> Can we add a new parameter to --pkg "*resql*" to run all the
>> reverse engineered test cases for all the modules, it just like parameter "
>> *all*" which is used to run all the regression tests. Following will be
>> the scenario if we add new parameter:
>>
>> - If we run --pkg all, run all the API and resql test cases.
>> - If we run --pkg <module list>, run the API and resql test cases for
>> the specified module list
>> - if we run --pkg resql, run all the resql test cases only.
>>
>> How about using the command line options '--only-resql', and '--no-resql'
> for the same?
> * If we run the test suite with '--only-resql', it should run only the
> test cases for the reverse engineering sql for all or selected packages
> specified by '--pkg'.
> * If we run the test suite with '--no-resql', no test cases for the
> reverse engineering sql should be running.
> * By default, test suite should run the test cases for reverse engineering
> sql too.
>
> NOTE: '--only-resql', and '--no-resql' must not be specified together.
>
> Let's leave the command line option '--pkg' for selecting the packages
> only.
>

Why add more options? I don't see why we can't think of these tests as just
another package. If that's really a problem, we could just rename it to
--tests or something.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company