Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1b9q4W-0001mP-8A for pgadmin-hackers@arkaria.postgresql.org; Mon, 06 Jun 2016 08:39:44 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1b9q4V-0000Cj-NF for pgadmin-hackers@arkaria.postgresql.org; Mon, 06 Jun 2016 08:39:43 +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 1b9q4U-0000CK-PY for pgadmin-hackers@postgresql.org; Mon, 06 Jun 2016 08:39:42 +0000 Received: from mail-qg0-x231.google.com ([2607:f8b0:400d:c04::231]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1b9q4M-0000ql-H5 for pgadmin-hackers@postgresql.org; Mon, 06 Jun 2016 08:39:41 +0000 Received: by mail-qg0-x231.google.com with SMTP id p34so38803806qgp.1 for ; Mon, 06 Jun 2016 01:39:34 -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=RM2n3+vpTv28EZUp14CuBrzN2op/IAHO0w0iZe/g3ek=; b=RM4w3b5Bp+1wHK2FZ5rMQcxwenx/xzbVzWO1m4C906zhwni2+uOfuKQ8eiIRhjSm5x NwR2gR63tegD1BAm6jA1qRAqfDAXWHXBSWiu1dnhm8n9oCUTJ8LCRV2oxvQ2DzZZaRxF W4cAMmMQ9Njk4/5pLuZDj+u7BlxSBKuMoOOGwievnpqopGAlZn3LveIt6+8yXBFb1pUX nglbXZkUCZPul8MGT37HzJdvTD9fAbNyXfyOSjLeWlBpOm8ToGSS5GLvEz+HuGaVYmlk 4qyGRpLPbukXBC/DCS/nFuIFUx/wGQ2QpyO2x5u9aYaYqXvJ6vwzvLcr587aiJeYfbNa Ij8A== 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=RM2n3+vpTv28EZUp14CuBrzN2op/IAHO0w0iZe/g3ek=; b=cwUvJlzL6+4/Fo9aXPO+aarkfPX5a1rOILjxaemwTTYpQNrBVMxORN8vHqvaFt7jhQ tqCwPdU1DIgJJMQlf2gocbrFUQgFPdkqdte3UDznWuV0DIqzHQqdsR5I4kHxj9OTHMlh Tf368saM37sNsvljeSya0dv6GWdg/67/ydI0kLxoWdSpecpsgAwda49XTTUPTfsf/XH4 mykGOM5GF9scVmFolQKk3Jh83Hzv1LbBtFnWGiF71ZmeV46ixgFivoY1Nq6EFYIKqaRZ z9DTs1uAaF/GxyNP14n2Wcen1b/vGSWXIWkW/bjoBaQ4gkmDQQymMD/2atc9/OsxxWVd 1S8w== X-Gm-Message-State: ALyK8tK0vCCkKsldEAsBzT5n3Pi/EOLifJPzSC97Mfjqc3eHQx0lY4tcNhgKidOX4xzTqzkAEX1MjY9FqEjh+38Z X-Received: by 10.141.28.204 with SMTP id f195mr14884704qhe.97.1465202372849; Mon, 06 Jun 2016 01:39:32 -0700 (PDT) MIME-Version: 1.0 Received: by 10.200.38.196 with HTTP; Mon, 6 Jun 2016 01:39:13 -0700 (PDT) In-Reply-To: References: From: Priyanka Shendge Date: Mon, 6 Jun 2016 14:09:13 +0530 Message-ID: Subject: Re: pgAdmin IV API test cases patch To: Dave Page Cc: pgadmin-hackers , Kanchan Mohitey Content-Type: multipart/alternative; boundary=001a11423d28f5488c0534980204 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 --001a11423d28f5488c0534980204 Content-Type: text/plain; charset=UTF-8 On 6 June 2016 at 14:03, Dave Page wrote: > Hi > > On Sun, Jun 5, 2016 at 6:56 PM, Priyanka Shendge > wrote: > > Hi, > > > > PFA patch for API test cases for tree nodes/modules. This patch does not > > include > > api tests for following modules: > > > > 1. Type > > 2. Table child nodes > > 3. FTS modules > > > > Kindly, review the same and let me know for any modification. > > I took a very quick look at the patch and immediately saw a serious > problem I'm afraid - you cannot hard-code paths like this: > > + if > os.path.isfile('/home/edb/Downloads/pgadmin4/web/regression/' > + 'parent_id.pkl'): > + exst_server_id = open('/home/edb/Downloads/pgadmin4/web' > + '/regression/''parent_id.pkl', 'rb') > > You need to dynamically generate such paths so this will work on any > machine. Look at line 19 of web/pgAdmin4.py to see how to get the > patch to the current file as an example. > Sure, i'll check and update accordingly. > > Also; despite this not being part of the end-user interface, please > try to follow the standards for messages, e.g. instead of: > > No event trigger(s) to update!!! > > use > > No event trigger(s) to update. > Noted. I will update for all applicable files. > > There is likely more to change of course, but please fix these issues > first. > Sure. Thank you. > > Thanks! > > -- > 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 --001a11423d28f5488c0534980204 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On 6 June 2016 at 14:03, Dave Page <dpage@pgadmin.org> w= rote:
Hi

On Sun, Jun 5, 2016 at 6:56 PM, Priyanka Shendge
<priyanka.shendge@e= nterprisedb.com> wrote:
> Hi,
>
> PFA patch for API test cases for tree nodes/modules. This patch does n= ot
> include
> api tests for following modules:
>
> 1. Type
> 2. Table child nodes
> 3. FTS modules
>
> Kindly, review the same and let me know for any modification.

I took a very quick look at the patch and immediately saw a serious<= br> problem I'm afraid - you cannot hard-code paths like this:

+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if os.path.isfile('/home/edb= /Downloads/pgadmin4/web/regression/'
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 'parent_id.pkl'):
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 exst_server_id =3D= open('/home/edb/Downloads/pgadmin4/web'
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 '/regressio= n/''parent_id.pkl', 'rb')

You need to dynamically generate such paths so this will work on any
machine. Look at line 19 of web/pgAdmin4.py to see how to get the
patch to the current file as an example.
Sure, i'l= l check and update accordingly.=C2=A0

Also; despite this not being part of the end-user interface, please
try to follow the standards for messages, e.g. instead of:

No event trigger(s) to update!!!

use

No event trigger(s) to update.
Noted. I will update fo= r all applicable files.=C2=A0

There is likely more to change of course, but please fix these issues first= .
Sure.=C2=A0

Thank you.

Thanks!

--
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 C= ompany
--001a11423d28f5488c0534980204--