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 1hcnCn-0007zi-98 for pgadmin-hackers@arkaria.postgresql.org; Mon, 17 Jun 2019 08:41:33 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1hcnCl-00027F-Vr for pgadmin-hackers@arkaria.postgresql.org; Mon, 17 Jun 2019 08:41:31 +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 1hcnCl-000278-Jq for pgadmin-hackers@lists.postgresql.org; Mon, 17 Jun 2019 08:41:31 +0000 Received: from mail-qt1-x841.google.com ([2607:f8b0:4864:20::841]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hcnCi-0003mL-Qw for pgadmin-hackers@postgresql.org; Mon, 17 Jun 2019 08:41:30 +0000 Received: by mail-qt1-x841.google.com with SMTP id h21so9680687qtn.13 for ; Mon, 17 Jun 2019 01:41:28 -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=9paDSf+VpgLIkYwH9BVxKaxUWH+qkv7XQXvKq3DvGuY=; b=M9PMzk6raVoyrzEpSbHO9JPMfQI98JJBmyJ3SLbVGCfGeci3Sd2HFDQcuEHBUxArcf KncKtRA9RzgwuxO5rFmGYTDuH0RB/GR+dyTaS0oLLVM6DUjxmWn5rdEGcWEEWKJJjdlH npEiPaQdrcbf/xduBAlg7qztyZLeImQsX1vN0uCr/MQ8crzJAXRvgH1x/RCs9ZVkjYbP LiFnA39nBZGONj4iQjVVPjuWA+ijxXCf6FOX6YNLRs3RsKFZbLGH5gMlSU4gvk2IPabW v45jMQGUc5vo8fp3zrROuScc9SSk25P3DijlKFZW47aONqFr5DVdjO/FkrlXdjuYUVaV 0fFg== 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=9paDSf+VpgLIkYwH9BVxKaxUWH+qkv7XQXvKq3DvGuY=; b=Js2i61PKJ4jMwHQ/bPmHYJciMa0lR4z26eZfgmcnRVY0CKQ1KXVFwRv4KsG6Hhh8On XAb+yq7A4HUDs5WtWikTYJ9cFydH3LTu4f4fdXh3g6+GY6aObLZwUua0NOeFpUv09fXY dRy2CAAZlECjeHF61zIhUV+fSTHD2FMEyxYJpgmmtwJL31gbEfMojjkerf0Kur91QUO6 dhoRFnKQBW+0MSPUQPQWWi2gWf5Hclo/QzwaBJrRCZdBX3m3FNZ+hFsURNC3uwp52vww FEgOKzDFWaLigDXxHC3OObXI0e1rjCw3faRmv2fKouERl/MhWh2hLs3ebo1bAqyBtSLN JrzQ== X-Gm-Message-State: APjAAAVxeNCzVfEsZZoooz4zMMI/dQO7daaAAvPHZYXgZxCrI8W4hY78 TxIgydCiT1y8nPS8xjrNUUE8GN8oZMYIo+cOsBuzCw== X-Google-Smtp-Source: APXvYqwMDZwXTU+sdkkTLqrjz6ju1Tdi+fkyaW+JvU/lIE102H+nU170gJOVWc1GW2ZiPNIrsgiwCP+gwDF9klIAWsU= X-Received: by 2002:a0c:9233:: with SMTP id a48mr19735720qva.66.1560760886899; Mon, 17 Jun 2019 01:41:26 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Akshay Joshi Date: Mon, 17 Jun 2019 14:11:15 +0530 Message-ID: Subject: Re: [pgAdmin4][Patch]: Feature #4202 Implement new framework to test Reverse Engineering SQL To: Dave Page Cc: Ashesh Vashi , pgadmin-hackers Content-Type: multipart/alternative; boundary="0000000000003e5eeb058b80f68a" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --0000000000003e5eeb058b80f68a Content-Type: text/plain; charset="UTF-8" Hi Dave On Mon, Jun 17, 2019 at 1:33 PM Dave Page wrote: > > > On Mon, Jun 17, 2019 at 8:19 AM Ashesh Vashi < > ashesh.vashi@enterprisedb.com> wrote: > >> >> On Mon, Jun 17, 2019 at 11:54 AM Akshay Joshi < >> akshay.joshi@enterprisedb.com> 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. >> > > 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* --0000000000003e5eeb058b80f68a Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi=C2=A0Dave

On Mon, Jun 17, 2019 at 1:33 = PM Dave Page <= dpage@pgad= min.org> wrote:


On Mon, Jun 17, 2019 at 8:= 19 AM Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

On Mon, Jun 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 = <aksh= ay.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> w= rote:
Hi Hackers

I have implemented the new test fram= ework to test the Reverse Engineering SQL. I have integrated it as a part o= f API/Regression test suite. It will work when we run all the=C2=A0test cas= es or module wise test case.

How it works: = Attached patch contains the generic framework to read all the JSON files fr= om the tests->version based (example 9.6_plus, 10_plus or default) fo= lder. Run all the test scenarios present in the JSON file in sequential= order.

Format of the JSON file is mentioned in &q= uot;<path_of_source>web/pgadmin/browser/server_groups/servers/databas= es/casts/tests/default/test.json"

For expecte= d SQL we will have following two options:
  • Provide the exp= ected sql in scenario itself as parameter "expected_sql" : &qu= ot;<SQL>".
  • Create a output file with any name in= the same directory where the JSON file resides and specify the parameter &= quot;expected_sql_file": "<name of the file>"
Attached patch contains both the above mentioned exa= mples.

Please review it.
<= div>
Nice!

A few comments:

- The scenario name should be "Reverse Engineered SQL = Test Cases"
- After the scenario name is output, can we outp= ut a \n so the next line isn't appended to the name?
<= /blockquote>
=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 obvious 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 tri= ed but facing issues when run only=C2=A0 "regression.re_sql.tests"= ;, will continue working on this.

=C2=A0 =C2=A0 =C2=A0Can we add a new parameter=C2=A0 to --pkg &quo= t;resql" to run all the reverse engineered test cases for all t= he 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 r= esql test cases 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 &#= 39;--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 spe= cified by '--pkg'.
* If we run the test suite with '-= -no-resql', no test cases for the reverse engineering sql should be run= ning.
* By default, test suite should run the test cases for reve= rse engineering sql too.

NOTE: '--only-resql&#= 39;, and '--no-resql' must not be specified together.
Let's leave the command line option '--pkg' for sel= ecting the packages only.=C2=A0

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

=C2=A0 =C2=A0As 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 --pk= g parameter it will import all the modules where "test." s= tring is present in the module name. If we provide the specific package lik= e "browser.server_groups.servers.databases.casts.tests" th= en it will import all the files of that module.=C2=A0

<= div>=C2=A0 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 test= s folder and get the JSON file. My question here is why do we need to separate th= e resql te= st cases? It would be good to have if they run along with the API test case= for all or specified module.

=C2=A0But 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 li= ke I suggest "--pkg resql" or suggested by Ashesh "--only-resql".=C2=A0=C2=A0
<= br>
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

E= nterpriseDB UK: h= ttp://www.enterprisedb.com
The Enterprise PostgreSQL Company


--
Thanks & Regards
=
Akshay Joshi
Sr. Software A= rchitect
EnterpriseDB Software I= ndia Private Limited
Mobile: +91 976-788-8246<= br>
--0000000000003e5eeb058b80f68a--