Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bH7t9-0008Ls-Kc for pgadmin-hackers@arkaria.postgresql.org; Sun, 26 Jun 2016 11:06:07 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1bH7t8-00047m-Eq for pgadmin-hackers@arkaria.postgresql.org; Sun, 26 Jun 2016 11:06:06 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1bH7su-0003u3-UZ for pgadmin-hackers@postgresql.org; Sun, 26 Jun 2016 11:05:53 +0000 Received: from mail-qk0-x229.google.com ([2607:f8b0:400d:c09::229]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1bH7sn-0007tK-9f for pgadmin-hackers@postgresql.org; Sun, 26 Jun 2016 11:05:51 +0000 Received: by mail-qk0-x229.google.com with SMTP id p10so183999314qke.3 for ; Sun, 26 Jun 2016 04:05:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=2tH2fqBJO6arJlOo3aMpRxJNjv2pmftPxuAaNOHWNi8=; b=rb1kEmdeK5BwVA4OiZv+ArPVtn7Huj/jgDwO8aXpqEZLHa0HPsBHYz5IR3Ov42xyAA Jz+IqYFSHHYKtx+RLWxsYy1FurZedqjEs9/i05Kylk672bKg6GPKZKrszU2uGwXt7jrI uKWcfYgE9LZstQR4BRVDB2lyPNfa0TojyfIb5s+7jDTcXiTmxEJnTO8Mjn0uRLm5cGfb /EUq8XKO6VhwHuXRmGJtFGkthFM8ZEfc9DhBhq073/alf7eunM/OqKpP1w9ustMver/G TjvJcCnj8hHDOVYHd5kuQApxGLw7JDCFyhXv9EVh6R8jkSucyjdfNSp8/NBny8azb6u9 r6Jg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=2tH2fqBJO6arJlOo3aMpRxJNjv2pmftPxuAaNOHWNi8=; b=b2bjWz+RH7W2awkuJm0sLJ/SrclmIIQPxYvZ1n7EXgtPOEsIdCcD0DE5rm1+NAmyig n045fpxEzyExB3eap5kMqV9gAoaLM6NA71TWojUDQqvSaVNovYMo33VbGa6/arR6S3oH 4Ly6mkMYtWkOJVGa1SrK1t/56pp1C1DHdXjDDIZMWkb+3r/jU1bP2LDtSnePutsuVK7+ yhViJYyW84rBqD8SwDNvvPkRPv6/OLTcuj4mSSj/2/0VEH5Zp7c5t6wjO0oAr34QDAqx gipcY4gQOuYn33dwOveTlBU5vURkPc8y3SYYkpjGSlNLk+DW+R6+txdHW7XXxqIkB7X3 rSKQ== X-Gm-Message-State: ALyK8tI93uPHwWargNe9On+zUeH13srPknDrjYUbGUN5nXwMm9CwCs5U4T+mmcK5tAKc3F+bSmDc7KPXLHbKGQQt X-Received: by 10.55.212.147 with SMTP id s19mr9027427qks.64.1466939144008; Sun, 26 Jun 2016 04:05:44 -0700 (PDT) MIME-Version: 1.0 Received: by 10.237.45.165 with HTTP; Sun, 26 Jun 2016 04:05:24 -0700 (PDT) In-Reply-To: References: From: Priyanka Shendge Date: Sun, 26 Jun 2016 16:35:24 +0530 Message-ID: Subject: Re: pgAdmin IV API test cases patch To: Dave Page Cc: pgadmin-hackers , Kanchan Mohitey Content-Type: multipart/alternative; boundary=001a1149af4a960d7505362c623e X-Pg-Spam-Score: -2.6 (--) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgadmin-hackers Precedence: bulk Sender: pgadmin-hackers-owner@postgresql.org --001a1149af4a960d7505362c623e Content-Type: text/plain; charset=UTF-8 On 24 June 2016 at 16:17, Dave Page wrote: > Hi > > On Thu, Jun 23, 2016 at 2:41 PM, Priyanka Shendge > wrote: > > > > > > On 15 June 2016 at 15:05, Priyanka Shendge > > wrote: > >> > >> Thanks a lot Dave. > >> > >> On 15 June 2016 at 14:09, Dave Page wrote: > >>> > >>> Hi > >>> > >>> On Thu, Jun 9, 2016 at 1:37 PM, Priyanka Shendge > >>> wrote: > >>> > Hi Dave, > >>> > > >>> > PFA updated patch. I have made changes suggested by you. > >>> > > >>> > Kindly, review and let me know for more changes. > >>> > >>> OK, I got a bit further this time, but not there yet. > >>> > >>> 1) The patch overwrote my test_config.json file. That should never > >>> happen (that file shouldn't be in the source tree). > >>> test_config.json.in should be the file that's included in the patch. > >> > >> > >> OK. > >>> > >>> > >>> 2) The updated test_config.json file is huge. > > > > > > Current configuration file web/regression/test_config.json contains test > > data(credentials) for each tree node; > > which is used while adding and updating the respective node. > > Why would we need that? Each node file (e.g. test_db_add.py and test_db_put.py) uses respective credentials from test_config.json while execution. We should have just one set of credentials for > everything. > Let me know if my understanding is clear: Should i keep basic credentials of each node (database, schema) into test_config.json instead taking care of each field? > > >>> > >>> I should only need to > >>> define one or more connections, then be able to run the tests. If you > >>> need to keep configuration info for "advanced users", let's put it in > >>> a different file to avoid confusing/scaring everyone else. Maybe split > >>> it into config.json for the stuff the user needs to edit > >>> (config.json.in would go in git), and test_config.json for the test > >>> configuration. > > > > > > Should i keep login and server credentials into > > web/regression/test_config.json file and > > put respective node details into config.json file of respective node's > tests > > directory? > > Not if you expect users to need to edit them - and if not, why are the > values not just hard-coded? > > > e.g. for database node: > > I'll create config.json file into .../databases/tests/ directory > > put database add and update credentials into config.json > > The key here is to make it simple for users. > > - To run the default tests, they should be able to copy/edit a simple > file, and just add database server details for the server to run > against. > > - If we have configurable tests (because making them configurable adds > genuine value), then we can use an "advanced" config file to allow the > user to adjust settings as they want. > > In the simple case, the user should be able to run the tests > successfully within a minute or two from starting. > > In designing the layout for files etc, remember the following: > > - Users should never edit a file that is in our source control. That's > why we have .in files that we expect them to copy. > > - Unless they're an advanced user, they shouldn't need to copy the > config file for advanced options. That means that the tests should > have defaults that match what is in the template advanced config file > (or, the tests could read advanced.json.in if advanced.json doesn't > exist, though that does seem a little icky). Of course, those are > example filenames, not necessarily what you may choose. > > -- > Dave Page > Blog: http://pgsnake.blogspot.com > Twitter: @pgsnake > > EnterpriseDB UK: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > -- Best, Priyanka EnterpriseDB Corporation The Enterprise PostgreSQL Company --001a1149af4a960d7505362c623e Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On 24 June 2016 at 16:17, Dave Page <dpage@pgadmin.org> = wrote:
Hi

On Thu, Jun 23, 2016 at 2:41 PM, Priyanka Shendge
<p= riyanka.shendge@enterprisedb.com> wrote:
>
>
> On 15 June 2016 at 15:05, Priyanka Shendge
> <priyanka.shen= dge@enterprisedb.com> wrote:
>>
>> Thanks a lot Dave.
>>
>> On 15 June 2016 at 14:09, Dave Page <dpage@pgadmin.org> wrote:
>>>
>>> Hi
>>>
>>> On Thu, Jun 9, 2016 at 1:37 PM, Priyanka Shendge
>>> <priya= nka.shendge@enterprisedb.com> wrote:
>>> > Hi Dave,
>>> >
>>> > PFA updated patch. I have made changes suggested by you.<= br> >>> >
>>> > Kindly, review and let me know for more changes.
>>>
>>> OK, I got a bit further this time, but not there yet.
>>>
>>> 1) The patch overwrote my test_config.json file. That should n= ever
>>> happen (that file shouldn't be in the source tree).
>>> test_config.json.in should be the file that's include= d in the patch.
>>
>>
>> OK.
>>>
>>>
>>> 2) The updated test_config.json file is huge.
>
>
> Current configuration file web/regression/test_config.json contains te= st
> data(credentials) for each tree node;
> which is used while adding and updating the respective node.

Why would we need that?

Each node f= ile (e.g. test_db_add.py and test_db_put.py) uses respective credentials = =C2=A0from=C2=A0
test_config.json while execution.
=
We should have just one set of credentials for everything.

Let me know if my understan= ding is clear:
=C2=A0=C2=A0
Should i keep basic credent= ials of each node (database, schema) into test_config.json
instea= d =C2=A0taking care of each field?

>>>
>>> I should only need to
>>> define one or more connections, then be able to run the tests.= If you
>>> need to keep configuration info for "advanced users"= , let's put it in
>>> a different file to avoid confusing/scaring everyone else. May= be split
>>> it into config.json for the stuff the user needs to edit
>>> (config.json.in would go in git), and test_config.json for t= he test
>>> configuration.
>
>
> Should i keep login and server credentials into
> web/regression/test_config.json file and
> put respective node details into config.json file of respective node&#= 39;s tests
> directory?

Not if you expect users to need to edit them - and if not, why are t= he
values not just hard-coded?

> e.g. for database node:
> I'll create config.json file into .../databases/tests/ directory > put database add and update credentials into config.json

The key here is to make it simple for users.

- To run the default tests, they should be able to copy/edit a simple
file, and just add database server details for the server to run
against.

- If we have configurable tests (because making them configurable adds
genuine value), then we can use an "advanced" config file to allo= w the
user to adjust settings as they want.

In the simple case, the user should be able to run the tests
successfully within a minute or two from starting.

In designing the layout for files etc, remember the following:

- Users should never edit a file that is in our source control. That's<= br> why we have .in files that we expect them to copy.

- Unless they're an advanced user, they shouldn't need to copy the<= br> config file for advanced options. That means that the tests should
have defaults that match what is in the template advanced config file
(or, the tests could read advanced.json.in if advanced.json doesn't exist, though that does seem a little icky). Of course, those are
example filenames, not necessarily what you may choose.

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

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



--
=
Best,
Priyanka

EnterpriseDB Corporation
The Enterprise PostgreSQL Co= mpany
--001a1149af4a960d7505362c623e--