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

Hi Dave

On Mon, Jun 17, 2019 at 1:33 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:

>
>
> 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.
>

As I mentioned in my previous email, this is not a regular
package/module in pgadmin directory. We have kept it in regression folder.
With current implementation if we provide "all" as a --pkg parameter it
will import all the modules where "*test.*" string is present in the module
name. If we provide the specific package like "
*browser.server_groups.servers.databases.casts.tests*" then it will import
all the files of that module.

So here problem is if we specify "python runtests.py --pkg
*regression.re_sql.tests*" we don't have list of all the module to iterate
over the *tests* folder and get the JSON file. My question here is why do
we need to separate the resql test cases? It would be good to have if they
run along with the API test case for all or specified module.

But if we will have to support it than we should have one option to
identify that we need to run only *re_sql* for all the modules. That we can
achieve by any options like I suggest "--pkg resql" or suggested by Ashesh
"--only-resql".

>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

--
*Thanks & Regards*
*Akshay Joshi*

*Sr. Software Architect*
*EnterpriseDB Software India Private Limited*
*Mobile: +91 976-788-8246*