Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bUvns-00063c-BP for pgadmin-hackers@arkaria.postgresql.org; Wed, 03 Aug 2016 13:01: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 1bUvnr-0008Jp-UP for pgadmin-hackers@arkaria.postgresql.org; Wed, 03 Aug 2016 13:01:43 +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 1bUvnd-00085U-PO for pgadmin-hackers@postgresql.org; Wed, 03 Aug 2016 13:01:30 +0000 Received: from mail-qt0-x236.google.com ([2607:f8b0:400d:c0d::236]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1bUvnW-0000yt-4J for pgadmin-hackers@postgresql.org; Wed, 03 Aug 2016 13:01:28 +0000 Received: by mail-qt0-x236.google.com with SMTP id 52so141920636qtq.3 for ; Wed, 03 Aug 2016 06:01:21 -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=mnCpZxFZrNYPurBThDMq2eVjN98LTKwm9eHZOF8ZNyI=; b=GaRLa1zrqNuUkSE/CtwoU+qC9rabEiyFdLoxdQzN4/dXT6RxR2DchFbd2clf8tSIaD z4RHp3qy+/8l5pLl5Bz1OrMvYtsL/3VfO7H2V8D5Y6TZlJnsKet1apcPQjIU5yb4hvdd VlK4hltRB8rHSZKsdOkpgxgUpB4XdQfFkMXw2X6lHkgVtfporlRAbteLu6mS5dNwAet8 hNEqDL5hXyp/1bpmfug+hsvGjMydMzi65ouhKxDoyMYQLKaN5DI5waXzQfqlrv9IMBSL pHrP2Come8diJFk2Ym8ql9YMTEuS9SpTJDPbTcGOaJ8uu/Xa7BtVsAGGwWHmr1zniptr Uz/Q== 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=mnCpZxFZrNYPurBThDMq2eVjN98LTKwm9eHZOF8ZNyI=; b=Mo2qRjN5hFhP7OtKDT4Tgz/YJk7kIjV7Eg87sAjvS0VYxxHRU1iC93UE3IBAeptkAU teEnA2GA3TZpV+q+LwgI2zV06nH4fiY8Tt46xj1km5H12D0oSxbFXaoIgTusKCiJORvY oikYbSwxGMOSbSWP5WoHq1KVKy/kSx3sH/hZyfntIb28YDoEix85imL7JeunWr+qmCqn pTOiFBoicKzbtfc7XtK2XbzQixXlQwHcJaW0tet555ytcy4yHZMrV/7x7C/u92vyygl1 cXUL2ygImJifZyTAhguLaGfXAhW1mgBIRvfTOBhXkOrXFGDgDFKVXCKXXEK49XVjbSIU u/9A== X-Gm-Message-State: AEkooutL0iUHdHcwxKm7kovlkgx94zztRAi1xOqop9Ia50pzBRkzfM9Pv672m4YGy86P9sZlFeKVzJ5gWV06s9vo X-Received: by 10.200.51.251 with SMTP id d56mr112278234qtb.36.1470229281057; Wed, 03 Aug 2016 06:01:21 -0700 (PDT) MIME-Version: 1.0 Received: by 10.200.41.90 with HTTP; Wed, 3 Aug 2016 06:01:19 -0700 (PDT) In-Reply-To: References: From: Navnath Gadakh Date: Wed, 3 Aug 2016 18:31:19 +0530 Message-ID: Subject: Re: pgAdmin IV : Unittest modular patch To: Dave Page Cc: pgadmin-hackers , Kanchan Mohitey Content-Type: multipart/alternative; boundary=001a1134f2ec09426c05392a6ea1 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 --001a1134f2ec09426c05392a6ea1 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Hi Dave, Thanks for clarification. On Wed, Aug 3, 2016 at 2:45 PM, Dave Page wrote: > Hi Navnath > > On Tue, Aug 2, 2016 at 3:58 PM, Navnath Gadakh > wrote: > > Hi Dave, > > Please find the attached patch. > > This patch includes: > > 1. API test cases for Roles & Tablespaces node(Completed nodes: server > > groups, servers, databases) > > You can run test-suite using following command > > for roles node > > python regression/runtests.py --pkg > > browser.server_groups.servers.roles > > for tablespaces node > > python regression/runtests.py --pkg > > browser.server_groups.servers.tablespaces > > for all nodes > > python regression/runtests.py > > You can also test with multiple servers. > > 2. Delete database code in some of the missed test files. > > 3. Added advanced configurations in test_advanced_config.json.in for > roles & > > tablespaces. So, accordingly you need change the file > > test_advanced_config.json > > 4. Added one test user credentials in test_config.json.in to test the > =E2=80=98valid > > password=E2=80=99 test case which is present in > > browser/tests/test_change_password.py > > Why test user credentials in test_config.json.in? > > Currently, I am getting =E2=80=98UnicodeDecodeError=E2=80=99= when I run > > test-suite(runtests.py) with existing code. I already explained the > detail > > about this error in RM(#1521). I am creating test user to test the =E2= =80=98valid > > password=E2=80=99 test case. > > The tablespace test is one that I think is going to cause us problems > - we really can't have a hard-coded path in the config; > Understood. > > - It might need to be different for each server being tested > Current patch supports multiple servers if you add > tablespace configurations for multiple servers. In the current patch, it'= s > only for one server. > For multiple servers you need to add multiple tablespace configuration: "test_tablespc_credentials":[{ "test_tblspace_name": "test_tablespace", "test_spc_location": "/Library/PostgreSQL/9.6/data", "test_spc_opts": [], "test_spc_user": "XXXXX" }, "test_tblspace_name": "test_tablespace1", "test_spc_seclable": [], "test_spc_location": "/Library/PostgrePlus/9.4AS/data", "test_spc_opts": [], "test_spc_user": "YYYYY" }] Anyhow, we are not going to use a hard coded path. - It is very likely to be different for each user > > I think what we need to do is: > > - Skip the tests if the server is not connected via a Unix domain > socket (hostname starts with /), 127.0.0.1 or ::1. > Understood. > > - Add per-server configuration options for the tablespace path and > service account under which the database server runs. These values > should default to /tmp and postgres for a PostgreSQL server, and edb > for PPAS. > Ok. For server connected via Unix domain socket. As per my understanding modified configuration should be, "tablespc_credentials":[{ "tblspace_name": "tablespace1", "testspc_seclable": [], "spc_acl": [ { "grantee":"postgres", "grantor":"postgres", "privileges":[ { "privilege_type":"C", "privilege":true, "with_grant":false } ] } ], "spc_location": "/tmp", "spc_opts": [], "spc_user": "postgres", "service_account": "test1_user" },{ "tblspace_name": "tablespace2", "spc_seclable": [], "spc_acl": [ { "grantee":"enterprisedb", "grantor":"enterprisedb", "privileges":[ { "privilege_type":"C", "privilege":true, "with_grant":false } ] } ], "spc_location": "/tmp", "spc_opts": [], "spc_user": "enterprisedb", "service_account": "test2_user" }] Please correct me if I am wrong. > > - Otherwise; assume the server is on the local machine, and: > - Create /$tblspace_path// > - chown $service_account /$tblspace_path// > - Run the tests > We can't set the owner for a tablespace path directory using normal user, the user should be 'root' Say, I am logged in my machine as a 'abc' user & I have installed PostgreSQL9.5 with 'postgres' user. It won't run chown postgres /$tablespace_path_for_PostgreSQL9.5 / > > Thoughts? > > I'd also suggest another couple of changes: > > - Remove the test_ prefix from the values in the config files. It > doesn't really help in the code as there you'll always have the data > in a variable anyway, e.g. adv_config_data["spc_location"] instead of > adv_config_data["test_spc_location"]. > > - I think we should fall back to using test_advanced_config.json.in if > the user hasn't made their own copy, e.g. > > try: > with open(CURRENT_PATH + '/test_advanced_config.json') as data_file: > advanced_config_data =3D json.load(data_file) > except: > with open(CURRENT_PATH + '/test_advanced_config.json.in') as > data_file: > advanced_config_data =3D json.load(data_file) Understood. > > > -- > Dave Page > VP, Chief Architect, Tools & Installers > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > > Blog: http://pgsnake.blogspot.com > Twitter: @pgsnake > --=20 Thanks, Navnath Gadakh Software Engineer EnterpriseDB Corporation Mobile: +91 9975389878 --001a1134f2ec09426c05392a6ea1 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hi Dave,
=C2=A0 =C2=A0 =C2=A0Thanks for clarification.=

On Wed, Aug 3= , 2016 at 2:45 PM, Dave Page <dave.page@enterprisedb.com><= /span> wrote:
Hi Navnath

On Tue, Aug 2, 2016 at 3:58 PM, Navnath Gadakh
<navnath.gadakh@enter= prisedb.com> wrote:
> Hi Dave,
>=C2=A0 =C2=A0 =C2=A0 Please find the attached pa= tch.
> This patch includes:
> 1. API test cases for Roles & Tablespaces node(Completed nodes: se= rver
> groups, servers, databases)
>=C2=A0 =C2=A0 =C2=A0You can run test-suite using following command
>=C2=A0 =C2=A0 =C2=A0 for roles node
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 python regressi= on/runtests.py --pkg
> browser.server_groups.servers.roles
>=C2=A0 =C2=A0 =C2=A0 for tablespaces node
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 python regressi= on/runtests.py --pkg
> browser.server_groups.servers.tablespaces
>=C2=A0 =C2=A0 =C2=A0 for all nodes
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 python regressi= on/runtests.py
>=C2=A0 =C2=A0 =C2=A0 You can also test with multiple=C2=A0 servers.
> 2. Delete database code in some of the missed test files.
> 3. Added advanced configurations in test_advanced_config.jso= n.in for roles &
> tablespaces. So, accordingly you need change the file
> test_advanced_config.json
> 4. Added one test user credentials in test_config.json.in to test= the =E2=80=98valid
> password=E2=80=99 test case which is present in
> browser/tests/test_change_password.py
>=C2=A0 =C2=A0 =C2=A0Why test user credentials in test_config.json.in?
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Currently, I am getting =E2= =80=98UnicodeDecodeError=E2=80=99 when I run
> test-suite(runtests.py) with existing code. I already explained the de= tail
> about this error in RM(#1521). I am creating test user to test the =E2= =80=98valid
> password=E2=80=99 test case.

The tablespace test is one that I think is going to cause us pr= oblems
- we really can't have a hard-coded path in the config;
=C2=A0 =C2=A0 =C2=A0 Understood.=C2=A0

- It might need to be different for each server being tested
=C2=A0 Current patch supports multiple servers if you add tablespace=C2=A0c= onfigurations for multiple servers.=C2=A0In the current patch, it's onl= y for one server.
=C2=A0 =C2=A0 =C2=A0 For multiple se= rvers you need to add multiple tablespace=C2=A0configuration:
=C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "test_tablespc_credentials&q= uot;:[{
=C2=A0 =C2=A0 "test_tblspace= _name": "test_tablespace",
=C2=A0 =C2=A0 "test_spc_location": "/Library/PostgreSQL/9.6= /data",
=C2=A0 =C2=A0 "test_spc= _opts": [],
=C2=A0 =C2=A0 "test= _spc_user": "XXXXX"
<= span style=3D"font-size:14px">=C2=A0 },
= =C2=A0 "test_tblspace_name": "test_tablespace1",=
=C2=A0 =C2=A0 "test_spc_seclable": []= ,
=C2=A0 =C2=A0 "test_spc_location&q= uot;: "/Library/PostgrePlus/9.4AS/data",
= =C2=A0 =C2=A0 "test_spc_opts": [],
<= div>=C2=A0 =C2=A0 "test_spc_user": "= YYYYY"

- It is very likely to be different for each user

I think what we need to do is:

- Skip the tests if the server is not connected via a Unix domain
socket (hostname starts with /), 127.0.0.1 or ::1.

- Add per-server configuration options for the tablespace path and
service account under which the database server runs. These values
should default to /tmp and postgres for a PostgreSQL server, and edb
for PPAS.
=C2=A0 =C2=A0 "spc_location": "/tmp",
=C2=A0 },{<= /div>
=C2=A0 =C2=A0 "tblspace_name": "tablespa= ce2",
=C2=A0 =C2=A0 "spc_seclab= le": [],
=C2=A0 =C2=A0 "spc_acl= ": [
=C2=A0 =C2=A0 =C2=A0 {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 "grantee":&= quot;enterprisedb",
=C2=A0 =C2=A0 = =C2=A0 =C2=A0 "grantor":"enterprisedb",
=C2=A0 =C2=A0 =C2=A0 =C2=A0 "privileges":[
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 {=
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "= ;privilege_type":"C",
=C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "privilege":true,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "w= ith_grant":false
=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 "spc_location": "/tmp",
=C2=A0 =C2=A0 "spc_opts": [],
=C2=A0 =C2=A0 "spc_user": "enterprisedb&= quot;,
=C2=A0 =C2=A0 "service_account": "test2_user&q= uot;
=C2=A0 }]
=
=C2=A0 =C2=A0 =C2=A0 =C2=A0 Please correct me if I am wrong.

- Otherwise; assume the server is on the local machine, and:
=C2=A0 - Create /$tblspace_path/<random_string>/
=C2=A0 - chown $service_account /$tblspace_path/<random_string>/
=C2=A0 - Run the tests
=C2=A0 =C2=A0 =C2=A0 We can'= ;t set the owner for=C2=A0a tablespace=C2=A0path directory=C2=A0using norma= l user, the user should be 'root'
=C2=A0 =C2=A0 =C2=A0 Sa= y, I am logged in my machine as a 'abc' user =C2=A0& I have ins= talled PostgreSQL9.5 with 'postgres' user.
=C2=A0 =C2=A0 = =C2=A0 It won't run=C2=A0chown post= gres=C2=A0/$tablespace_path_for_PostgreSQL9.5/<random_string>=C2=A0

Thoughts?

I'd also suggest another couple of changes:

- Remove the test_ prefix from the values in the config files. It
doesn't really help in the code as there you'll always have the dat= a
in a variable anyway, e.g. adv_config_data["spc_location"] instea= d of
adv_config_data["test_spc_location"].

- I think we should fall back to using test_advanced_config.json.= in if
the user hasn't made their own copy, e.g.

try:
=C2=A0 =C2=A0 with open(CURRENT_PATH + '/test_advanced_config.json'= ) as data_file:
=C2=A0 =C2=A0 =C2=A0 =C2=A0 advanced_config_data =3D json.load(data_file) except:
=C2=A0 =C2=A0 with open(CURRENT_PATH + '/test_advanced_config= .json.in') as data_file:
=C2=A0 =C2=A0 =C2=A0 =C2=A0 advanced_config_data =3D json.load(data_file)
=C2=A0 =C2=A0 Understood.=C2=A0


--
Dave Page
VP, Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake



--
=
=
Thanks,
Navnath= =C2=A0Gadakh
Software Engineer
EnterpriseDB Corporation
<= /font>
Mobile: +91 9975389878=C2=A0

<= /font>

=
--001a1134f2ec09426c05392a6ea1--