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 1hcmbm-00060x-4P for pgadmin-hackers@arkaria.postgresql.org; Mon, 17 Jun 2019 08:03:18 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1hcmbl-00075r-0c for pgadmin-hackers@arkaria.postgresql.org; Mon, 17 Jun 2019 08:03:17 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hcmbk-00074V-Df for pgadmin-hackers@lists.postgresql.org; Mon, 17 Jun 2019 08:03:16 +0000 Received: from mail-io1-xd41.google.com ([2607:f8b0:4864:20::d41]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hcmbd-0000W0-4O for pgadmin-hackers@postgresql.org; Mon, 17 Jun 2019 08:03:14 +0000 Received: by mail-io1-xd41.google.com with SMTP id w25so19136225ioc.8 for ; Mon, 17 Jun 2019 01:03:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pgadmin.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=E4GkhrkX6ii6yFehDQvIWWtF37iQom2Vi471cf6JX20=; b=pQfVQJW9AukKWaH5Aa6fbtewZvCO7Uol+aYpA/44K2OuuUpGXPegTNv+rU8Bry2bgM dN0dcRuAYiDk/aWOnHCLptgsVIQPydkvRneqJToSzrp0ZlcUwddAIxxyOwfITMk/RhZE RKCqr7BpMYUYsIJPVSyZMFiKlpUqK16K2Lqsfxx+vQac+yAcjDOo2gxspMJzQ2Q2otUK t19r7E6litilAa+E1xEtxT+QMRjjcB0FkxjnaYiYcMeUg0lzXI7TZfV8RSJKOGApb1p2 SCQtuqY3RdpLrSDtYBOLV8kOJBCT8JWCilCQOLXjicFckhqlhx7JX+V9JOHJ4uHV5myJ 3UQg== 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=E4GkhrkX6ii6yFehDQvIWWtF37iQom2Vi471cf6JX20=; b=V8xVYb3UZtvU08aZynXKGL35CskrnVCXgbWl29CSlkZDmrq3aA53qVSdk0bcMwwW/U JqTwilbeNHmu12kbe0PCwt81UZALklNc6/wZxIDkDcExgvcj5WHOfAK2wyvdW2xfRcyY jXclfR1plWY3BCTuGHo7L3L2CJ3hY01Y4rQLOw9nomKvXH26wdGcaEWPtyU3Qz1EAzDR SLsughTVmnbeB8xkkms/XnEp4NMyiZH0MrX6Ir23iS7qAFDZq409yNNQ4mRisuzQ3pTu 39ZvhJfHzE08egIGNtaru/oJVeaHzFQIuBDVvTilRqJBHdv0uVI7t4LIMmDNSrqVuvE0 qxFg== X-Gm-Message-State: APjAAAWY805LjEK75b9JojoDPon2KvK4sTkQAOoyJ4FDZIUXx0jhwz8T fFQGO9e68LrG0i/3YG1pcsE9rg/THpe+1aKtItNabA== X-Google-Smtp-Source: APXvYqyDoalSWsZnbT5WyPGJnVRokPmijakd+SN9y4ITm/vyhcAVGbTEecK4dqEFgSnMpJW2qU0glmxUdp0spE1Yf8M= X-Received: by 2002:a5d:9d97:: with SMTP id 23mr25179752ion.204.1560758588258; Mon, 17 Jun 2019 01:03:08 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Dave Page Date: Mon, 17 Jun 2019 09:02:56 +0100 Message-ID: Subject: Re: [pgAdmin4][Patch]: Feature #4202 Implement new framework to test Reverse Engineering SQL To: Ashesh Vashi Cc: Akshay Joshi , pgadmin-hackers Content-Type: multipart/alternative; boundary="0000000000003bd0e8058b806d17" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --0000000000003bd0e8058b806d17 Content-Type: text/plain; charset="UTF-8" On Mon, Jun 17, 2019 at 8:19 AM Ashesh Vashi 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. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company --0000000000003bd0e8058b806d17 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Mon, Jun 17, 2019 at 8:19 AM Ashes= h Vashi <ashesh.vashi@e= nterprisedb.com> wrote:
On= Mon, Jun 17, 2019 at 11:54 AM Akshay Joshi <akshay.joshi@enterprisedb.com&g= t; wrote:
Hi=C2=A0Dave/Hackers

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


On Fri, Jun 14, 2019 at 1:59 PM Dave Page <dpage@pgadmin.org> wrote:
<= blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-l= eft:1px solid rgb(204,204,204);padding-left:1ex">
Hi

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

I have implemented th= e 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 th= e=C2=A0test cases or module wise test case.

How= it works: Attached patch contains the generic framework to read all th= e 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 fil= e 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"

<= div>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 w= ith any name in the same directory where the JSON file resides and specify = the parameter "expected_sql_file": "<name of the file&= gt;"
Attached patch contains both the abov= e mentioned examples.

Please review it.

Nice!

A few comm= ents:

- The scenario name should be "Reverse = Engineered SQL Test Cases"
- After the scenario name is outp= ut, can we output a \n so the next line isn't appended to the name?
=C2=A0 =C2=A0
=C2=A0 =C2=A0Will f= ix the above.=C2=A0
=
- How do we run only the r= e_sql tests? I tried the obvious ways (e.g.=C2=A0python runtests.py --pkg r= egression.re_sql.tests.test_resql.ReverseEngineeringSQLTestCase) but got er= rors. 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 cod= e. I have tried but facing issues when run only=C2=A0 "regression.re_s= ql.tests", will continue 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 ca= ses for all the modules, it just like parameter "all" whic= h is used to run all the regression tests. Following will be the scenario i= f 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=C2=A0
  • if= we run --pkg resql, run all the resql test cases only.=C2=A0 =C2=A0=C2=A0<= /li>
How about using the command li= ne 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 suit= e with '--no-resql', no test cases for the reverse engineering sql = should be running.
* By default, test suite should run the test c= ases for reverse engineering sql too.

NOTE: '-= -only-resql', and '--no-resql' must not be specified together.<= /div>

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

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 proble= m, we could just rename it to --tests or something.=C2=A0
<= br>
--
Dave Page
Blo= g: http://pgsnake= .blogspot.com
Twitter: @pgsnake

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