Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bK1sA-0004m2-Lw for pgadmin-hackers@arkaria.postgresql.org; Mon, 04 Jul 2016 11:17:06 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1bK1sA-0003xr-6t for pgadmin-hackers@arkaria.postgresql.org; Mon, 04 Jul 2016 11:17:06 +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_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1bK1s9-0003xl-7N for pgadmin-hackers@postgresql.org; Mon, 04 Jul 2016 11:17:05 +0000 Received: from mail-qk0-x22c.google.com ([2607:f8b0:400d:c09::22c]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1bK1s0-0000aM-8f for pgadmin-hackers@postgresql.org; Mon, 04 Jul 2016 11:17:04 +0000 Received: by mail-qk0-x22c.google.com with SMTP id e3so61305750qkd.0 for ; Mon, 04 Jul 2016 04:16:55 -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=PAyuTnjmFexw6ygo4dTVe3pLP0gX7OXK+wkJG0O3vBo=; b=wxnn6fyGQxXaVNzcN5UrVjVHZfauqXvIzewNLRIY+XnHIjoBW+71jMtljtEXpmhrA8 S+LdQm3KzhNhydivP75cqGaIqADAkW0G3Miu77u+ff9JIBZZhvJ3dOtAfr2tapU3fRh0 sdorkIkaWKQfcyKyPHJba13DIHubxYcsUdMTYQL8uQ3PgBsgchlQQ08VvtimgBhPE+AU A25Jf6i9rIZyxJBez8yzoMUeIaiUkSBJ0wYDfdnMgFG6sqrG5GG6ilK4kezklojtdCYw susy9cdXJ9K+VjAupnFPJvwDE3Jlz/bOp+KGr/XVTNhapS4FdGpjfjyOXyaZxE0qHcRA dtRA== 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=PAyuTnjmFexw6ygo4dTVe3pLP0gX7OXK+wkJG0O3vBo=; b=J1N0oDHAhbMiBpkeLM7wsob9yBsSdLeK4kUOw4JynNiCHt0C7fe/CQZvuJBm+V0ySX hPTlLjwmynGHQsxNqx3V1h4XcbNMj3VuaYoO0lUYoL925jDNwrfIL7EdESResoC3CCvq 6kiMdC8timku0ITXYPknlXn1UZGS5PAp1B4nrQwAzcoVSgOKz/LuQE2b0TE6RsPta3EH woV8jC2x+yj2H4PseKc4vp8nF8qQ/NSnOsQcJGcDR7FcoAapRT604CgUC6bsnCwGBiP5 vVxzAHr620HlCT5c3W3pDOQ5l1u+eIwCDl7/dhqRwBsCQTWgQmXGOnJC3UTVlIEpN2G/ bgJw== X-Gm-Message-State: ALyK8tJ6LXbQaKxs9wosN/HO64HZolQwtMcLSNv8qSrpVr8UcvkyDpnVZEWf3ZDo1lkUNDQr3FmSM88TOk3n8cPj X-Received: by 10.55.175.134 with SMTP id y128mr15556229qke.67.1467631013974; Mon, 04 Jul 2016 04:16:53 -0700 (PDT) MIME-Version: 1.0 Received: by 10.237.45.165 with HTTP; Mon, 4 Jul 2016 04:16:34 -0700 (PDT) In-Reply-To: References: From: Priyanka Shendge Date: Mon, 4 Jul 2016 16:46:34 +0530 Message-ID: Subject: Re: pgAdmin IV API test cases patch To: Dave Page Cc: pgadmin-hackers , Kanchan Mohitey Content-Type: multipart/alternative; boundary=94eb2c06e58e4007c10536cd7957 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 --94eb2c06e58e4007c10536cd7957 Content-Type: text/plain; charset=UTF-8 Hi Dave, I am unable to reproduce issue on my side; tried on Python 2.7 and Python 3.4. Could you please provide me DEBUG logs and test data using for database node? Thank you. On 30 June 2016 at 00:51, Dave Page wrote: > Hi, > > That's better. I tweaked a few things and fixed a bug related to > recent changes to the schema version config. Patch attached. > > However, there are still issues: > > 1) The testsuite doesn't run to completion. See the attached > stdout.txt and logger.txt files. > 2) stdout should only display the test summary - what tests are > currently running (and pass/fail), and a summary at the end - even if > there's a crash like I saw. > 3) The output log file should contain the full output, including > what's sent to stdout. > 4) The output advises the user to check ".../pgadmin4/web/regression". > This should be in the summary at the end, and should be corrected to > show the correct (full) path. > > Thanks. > > > On Wed, Jun 29, 2016 at 2:52 PM, Priyanka Shendge > wrote: > > Hi Dave, > > > > As per discussion over mail i have created separate config files for > > credentials and test data. > > > > PFA patch for same. Kindly, review and let me know for modifications. > > > > On 27 June 2016 at 15:10, Priyanka Shendge > > wrote: > >> > >> > >> > >> On 27 June 2016 at 13:24, Dave Page wrote: > >>> > >>> On Sun, Jun 26, 2016 at 12:05 PM, Priyanka Shendge > >>> wrote: > >>> > > >>> > > >>> > 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 test data from > >>> > test_config.json while execution. > >>> > >>> That doesn't answer my question - why do we need separate credentials > >>> for each node? > >> > >> > >> Sorry for typo, its test data not credentials. > >> > >>> > >>> > >>> >> 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? > >>> > >>> You should have one set of credentials that's used for the entire test > >>> run. > >> > >> > >> Sure. I'll separate the credentials and test data into 2 different > files. > >> So, a normal user can run the tests into one go after some minor > >> credentials changes. > >> And an advanced user can have an option to change the test data if he > >> wants. > >>> > >>> > >>> >> >>> 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 > >>> > >>> > >>> > >>> -- > >>> Dave Page > >>> Blog: http://pgsnake.blogspot.com > >>> Twitter: @pgsnake > >>> > >>> EnterpriseDB UK: http://www.enterprisedb.com > >>> The Enterprise PostgreSQL Company > >>> > >>> > >>> -- > >>> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) > >>> To make changes to your subscription: > >>> http://www.postgresql.org/mailpref/pgadmin-hackers > >> > >> > >> > >> > >> -- > >> Best, > >> Priyanka > >> > >> EnterpriseDB Corporation > >> The Enterprise PostgreSQL Company > > > > > > > > > > -- > > Best, > > Priyanka > > > > EnterpriseDB Corporation > > The Enterprise PostgreSQL Company > > > > -- > 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 --94eb2c06e58e4007c10536cd7957 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hi Dave,

I am unable to reproduce issue= on my side; tried on Python 2.7 and Python 3.4.
Could you please= provide me DEBUG logs and test data using for database node?=C2=A0

Thank you.

On 30 June 2016 at 00:51, Dave Page <dpage@pgadmin.= org> wrote:
Hi,

That's better. I tweaked a few things and fixed a bug related to
recent changes to the schema version config. Patch attached.

However, there are still issues:

1) The testsuite doesn't run to completion. See the attached
stdout.txt and logger.txt files.
2) stdout should only display the test summary - what tests are
currently running (and pass/fail), and a summary at the end - even if
there's a crash like I saw.
3) The output log file should contain the full output, including
what's sent to stdout.
4) The output advises the user to check ".../pgadmin4/web/regression&q= uot;.
This should be in the summary at the end, and should be corrected to
show the correct (full) path.

Thanks.


On Wed, Jun 29, 2016 at 2:52 PM, Priyanka Shendge
<priyanka.shendge@enterprisedb.com> wrote:
> Hi Dave,
>
> As per discussion over mail i have created separate config files for > credentials and test data.
>
> PFA patch for same. Kindly, review and let me know for modifications.<= br> >
> On 27 June 2016 at 15:10, Priyanka Shendge
> <priyanka.shen= dge@enterprisedb.com> wrote:
>>
>>
>>
>> On 27 June 2016 at 13:24, Dave Page <dpage@pgadmin.org> wrote:
>>>
>>> On Sun, Jun 26, 2016 at 12:05 PM, Priyanka Shendge
>>> <priya= nka.shendge@enterprisedb.com> wrote:
>>> >
>>> >
>>> > 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
>>> >> <priyanka.shendge@enterprisedb.com> wrote:
>>> >> >
>>> >> >
>>> >> > On 15 June 2016 at 15:05, Priyanka Shendge
>>> >> > <priyanka.shendge@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
>>> >> >>> <priyanka.shendge@enterprisedb.com> wrote:
>>> >> >>> > Hi Dave,
>>> >> >>> >
>>> >> >>> > PFA updated patch. I have made chan= ges suggested by you.
>>> >> >>> >
>>> >> >>> > Kindly, review and let me know for = more changes.
>>> >> >>>
>>> >> >>> OK, I got a bit further this time, but n= ot there yet.
>>> >> >>>
>>> >> >>> 1) The patch overwrote my test_config.js= on file. That should never
>>> >> >>> happen (that file shouldn't be in th= e source tree).
>>> >> >>> test_config.json.in should be the f= ile that's included in the
>>> >> >>> patch.
>>> >> >>
>>> >> >>
>>> >> >> OK.
>>> >> >>>
>>> >> >>>
>>> >> >>> 2) The updated test_config.json file is = huge.
>>> >> >
>>> >> >
>>> >> > Current configuration file web/regression/test_c= onfig.json contains
>>> >> > test
>>> >> > data(credentials) for each tree node;
>>> >> > which is used while adding and updating the resp= ective node.
>>> >>
>>> >> Why would we need that?
>>> >
>>> >
>>> > Each node file (e.g. test_db_add.py and test_db_put.py) u= ses respective
>>> > credentials test data=C2=A0 from
>>> > test_config.json while execution.
>>>
>>> That doesn't answer my question - why do we need separate = credentials
>>> for each node?
>>
>>
>> Sorry for typo, its test data not credentials.
>>
>>>
>>>
>>> >> 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, s= chema) into
>>> > test_config.json
>>> > instead=C2=A0 taking care of each field?
>>>
>>> You should have one set of credentials that's used for the= entire test
>>> run.
>>
>>
>> Sure.=C2=A0 I'll separate the credentials and test data into 2= different files.
>> So, a normal user can run the tests into one go after some minor >> credentials changes.
>> And an advanced user can have an option to change the test data if= he
>> wants.
>>>
>>>
>>> >> >>> 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 &quo= t;advanced users", let's put it
>>> >> >>> in
>>> >> >>> a different file to avoid confusing/scar= ing everyone else. Maybe
>>> >> >>> split
>>> >> >>> it into config.json for the stuff the us= er 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<= br> >>> >> > web/regression/test_config.json file and
>>> >> > put respective node details into config.json fil= e 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 .../databa= ses/tests/ directory
>>> >> > put database add and update credentials into con= fig.json
>>> >>
>>> >> The key here is to make it simple for users.
>>> >>
>>> >> - To run the default tests, they should be able to co= py/edit a simple
>>> >> file, and just add database server details for the se= rver to run
>>> >> against.
>>> >>
>>> >> - If we have configurable tests (because making them = configurable adds
>>> >> genuine value), then we can use an "advanced&quo= t; config file to allow the
>>> >> user to adjust settings as they want.
>>> >>
>>> >> In the simple case, the user should be able to run th= e tests
>>> >> successfully within a minute or two from starting. >>> >>
>>> >> In designing the layout for files etc, remember the f= ollowing:
>>> >>
>>> >> - Users should never edit a file that is in our sourc= e control. That's
>>> >> why we have .in files that we expect them to copy. >>> >>
>>> >> - Unless they're an advanced user, they shouldn&#= 39;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 adva= nced config file
>>> >> (or, the tests could read advanced.json.in if advan= ced.json doesn't
>>> >> exist, though that does seem a little icky). Of cours= e, those are
>>> >> example filenames, not necessarily what you may choos= e.
>>> >>
>>> >> --
>>> >> 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
>>>
>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>>
>>> --
>>> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
>>> To make changes to your subscription:
>>> http://www.postgresql.org/mailpref/pg= admin-hackers
>>
>>
>>
>>
>> --
>> Best,
>> Priyanka
>>
>> EnterpriseDB Corporation
>> The Enterprise PostgreSQL Company
>
>
>
>
> --
> Best,
> Priyanka
>
> EnterpriseDB Corporation
> The Enterprise PostgreSQL Company



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

--94eb2c06e58e4007c10536cd7957--