Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hclvA-0004Lz-Gl for pgadmin-hackers@arkaria.postgresql.org; Mon, 17 Jun 2019 07:19:16 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1hclv9-0003Xb-Cd for pgadmin-hackers@arkaria.postgresql.org; Mon, 17 Jun 2019 07:19:15 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hclv9-0003XU-4m for pgadmin-hackers@lists.postgresql.org; Mon, 17 Jun 2019 07:19:15 +0000 Received: from mail-oi1-x241.google.com ([2607:f8b0:4864:20::241]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hclv5-00034A-QA for pgadmin-hackers@postgresql.org; Mon, 17 Jun 2019 07:19:14 +0000 Received: by mail-oi1-x241.google.com with SMTP id w196so6273615oie.7 for ; Mon, 17 Jun 2019 00:19:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=wtsEGXZ3h904/cRpdwP2iidwO+SfP0y/GF4NXsF+gf4=; b=EFYMEK2WMNoAJFVU1D73/wpo5llxOcynHh03Z4RPsYXHXdy+6HvzXQFRKRO04mGj/I Rssb4h7zJXtl2C+r7KWP4etqtp6KRpTscHRDg5XEvHWiEwGoNeT+NA6oR3oCMdfMU20T wfZ2osS+uhQY+c+8nJMrQnTzhYzLsX51aeLZ36Wxb8p65+L9TPEjL5kqq9q8EYO4JalC +OYrdZhKz0Wk1vgw5LOes7viqMD5Jm+0RREaL3WolT2PAXpXrFve/WdccY48FJMhUAxT 60MbibcQByhGFtlZWvkBSOE3osxhRFNUKGvA2fvBchm5ELedw8jNwMsIK6PpUtJd2kQD Z9Fw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=wtsEGXZ3h904/cRpdwP2iidwO+SfP0y/GF4NXsF+gf4=; b=q06Nni7S/iabotGDnOirzptiT6oQmC7xBJ4WL+i5tZkSxlqlWwZpn3GYz1XMIXnC1k /ewpPmzboapddfUAIe9rUN1INXe5gTVO21OraaAVUKVUUYy8h2ujLvZ6aBF+4ttICEk6 Irla9jWVTnGi3rysb3XJptKYmiWR1OptBCSua9NS1ZS7KVZQJmUAA/Zm6LQWSAhv4/pM yl6cpglXpDhPaJyfRtD0luAbbJBFthR0zAA7DdU8ABowzDgt4MQDJPOJhrsoayDvdi51 ux055Wwn8TTRWo756sAkv6mGh3ehdfDnGKMi5AfJqpyWBVroDPx5MA6ZL0vvEeNwCQoh c3sg== X-Gm-Message-State: APjAAAVMpogfV4bVOdLs7/YW/W+aK5mVIMVjV0E4dzj2oqXqtS1WBfgN cXJFD7W4e+MWLs1uqOaO++eGd/Q6K+rOktWfWazOwg== X-Google-Smtp-Source: APXvYqwK2aqIiEOwG0V9ppxtD+iT2uqNgFj6QYVPr6/nhW0hHHLBWNUMqWOf4YjV6288ks5rtcsytxWPaywsN9/+TRQ= X-Received: by 2002:aca:3a03:: with SMTP id h3mr9453095oia.169.1560755949243; Mon, 17 Jun 2019 00:19:09 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Ashesh Vashi Date: Mon, 17 Jun 2019 12:48:58 +0530 Message-ID: Subject: Re: [pgAdmin4][Patch]: Feature #4202 Implement new framework to test Reverse Engineering SQL To: Akshay Joshi Cc: Dave Page , pgadmin-hackers Content-Type: multipart/alternative; boundary="000000000000efa8b6058b7fcf48" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --000000000000efa8b6058b7fcf48 Content-Type: text/plain; charset="UTF-8" On Mon, Jun 17, 2019 at 11:54 AM Akshay Joshi wrote: > Hi Dave/Hackers > > On Fri, Jun 14, 2019 at 6:10 PM Akshay Joshi < > akshay.joshi@enterprisedb.com> wrote: > >> >> >> On Fri, Jun 14, 2019 at 1:59 PM Dave Page wrote: >> >>> Hi >>> >>> On Thu, Jun 13, 2019 at 12:52 PM Akshay Joshi < >>> akshay.joshi@enterprisedb.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 >>>> "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" >>>> : ""*. >>>> - Create a output file with any name in the same directory where >>>> the JSON file resides and specify the parameter "*expected_sql_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 , 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. -- Thanks, Ashesh >> >>> - Once we have a way to run these tests only, please add a "make >>> check-resql" target to the Makefile. >>> - Can the expected output be formatted in the JSON such that it doesn't >>> use \n, but uses regular line breaks? That would make it easier to >>> copy/paste. >>> >> >> I have tried that during implementation, but JSON does not allow >> line-breaks. >> >>> >>> Thanks. >>> >>> -- >>> 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* >> > > > -- > *Thanks & Regards* > *Akshay Joshi* > > *Sr. Software Architect* > *EnterpriseDB Software India Private Limited* > *Mobile: +91 976-788-8246* > --000000000000efa8b6058b7fcf48 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable

=
On Mon, Ju= n 17, 2019 at 11:54 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi=C2=A0Dave/Hackers

On Fri, Jun 14, 2019 at 6:10 PM Akshay Joshi <akshay.joshi= @enterprisedb.com> wrote:


On Fri, Jun 14, 2019= at 1:59 PM Dave Page <dpage@pgadmin.org> wrote:
Hi

On Thu, Jun 13, = 2019 at 12:52 PM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
<= /div>
Hi = Hackers

I have implemented the new test framework to tes= t the Reverse Engineering SQL. I have integrated it as a part of API/Regres= sion test suite. It will work when we run all the=C2=A0test cases or module= wise test case.

How it works: Attached pat= ch contains the generic framework to read all the JSON files from the te= sts->version based (example 9.6_plus, 10_plus or default) folder. Ru= n 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/tes= ts/default/test.json"

For expected SQL we wil= l have following two options:
  • Provide the expected sql in= scenario itself as parameter "expected_sql" : "<SQL&g= t;".
  • Create a output file with any name in the same di= rectory where the JSON file resides and specify the parameter "expe= cted_sql_file": "<name of the file>"
=
Attached patch contains both the above mentioned examples.
=

Please review it.

Nice!

A few comments:

<= div>- The scenario name should be "Reverse Engineered SQL Test Cases&q= uot;
- After the scenario name is output, can we output a \n so t= he next line isn't appended to the name?
=
=C2=A0 =C2=A0
=C2=A0 =C2=A0Will fix the above.=C2=A0
- How do we run only the re_sql tests? I tried the ob= vious ways (e.g.=C2=A0python runtests.py --pkg regression.re_sql.tests.test= _resql.ReverseEngineeringSQLTestCase) but got errors. Please add an example= to web/regression/README.

=C2=A0 =C2=A0It 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=C2=A0 "regression.re_sql.tests", will contin= ue working on this.

=C2= =A0 =C2=A0 =C2=A0Can we add a new parameter=C2=A0 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 ca= ses for the specified module list=C2=A0
  • if we run --pkg resql, run = all the resql test cases only.=C2=A0 =C2=A0=C2=A0
How about using the command line options '--only-re= sql', and '--no-resql' for the same?
* If we run the = test suite with '--only-resql', it should run only the test cases f= or the reverse engineering sql for all or selected packages specified by &#= 39;--pkg'.
* If we run the test suite with '--no-resql= 9;, no test cases for the reverse engineering sql should be running.
<= div>* By default, test suite should run the test cases for reverse engineer= ing sql too.

NOTE: '--only-resql', and = 9;--no-resql' must not be specified together.

= Let's leave the command line option '--pkg' for selecting the p= ackages only.=C2=A0

-- Thanks, Ashesh
=
=C2=A0
- Once we have a way to run these tests only, please add a &quo= t;make check-resql" target to the Makefile.
- Can the expect= ed output be formatted in the JSON such that it doesn't use \n, but use= s regular line breaks? That would make it easier to copy/paste.
=

=C2=A0 =C2=A0 I have tried that duri= ng implementation, but JSON does not allow line-breaks.

Thanks.

--
Dave Page
Blog: http://pgsnake.blogspot.com<= br>Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterpri= se PostgreSQL Company


--
Thanks & Regards
Akshay Joshi
<= div>Sr. Software Arch= itect
EnterpriseDB Soft= ware India Private Limited
Mobile: +91 976-788-8246


--
=
Thanks & Reg= ards
Akshay Joshi
Sr. Software Architect
EnterpriseDB Software India Private Limited=
Mobile: +91 976-788-8246
--000000000000efa8b6058b7fcf48--