Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bVEvt-0005yI-OM for pgadmin-hackers@arkaria.postgresql.org; Thu, 04 Aug 2016 09:27:18 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1bVEvt-0007ZR-9w for pgadmin-hackers@arkaria.postgresql.org; Thu, 04 Aug 2016 09:27:17 +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 1bVEvr-0007ZI-S3 for pgadmin-hackers@postgresql.org; Thu, 04 Aug 2016 09:27:16 +0000 Received: from mail-qk0-x22f.google.com ([2607:f8b0:400d:c09::22f]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1bVEvo-0006wm-0O for pgadmin-hackers@postgresql.org; Thu, 04 Aug 2016 09:27:14 +0000 Received: by mail-qk0-x22f.google.com with SMTP id x185so33282740qkc.2 for ; Thu, 04 Aug 2016 02:27:11 -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=BT3r4bHDuxSPFMkrDBkG0N1vqRbSQvhL5jxint7QG50=; b=lFGUhlsZigy/D/1NFw2Nu7ZL+fK8NrGfWruJwnjkR/NROO8Yzt3p+4anJgBGBKquUM P1jB9WFnOybEiJdjop90QVWPqZOKFJ5e4t/u1hj2N/ddcP5h3KiM7atfYXsuYhiUCvwE PWmdgqIQkjSNs8Pjj+jA1GWGNdPC3SdMpumeOz/NNBf36joPFDCVwyNCfqNdZBt9F0m2 8e0VfCB6sW+xrLf2i2N6ChBpXNieLzmeAri3a/wt7HMSV4/d2zrqIhLkMJc477WxLBvp l7OBmhcVaXQm0qd70IAU5Wtv9rTD0Pk2v+LOlvlqRN/vCo4TmYD4bnZ/pAoo8UXpy/7W 03Gw== 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=BT3r4bHDuxSPFMkrDBkG0N1vqRbSQvhL5jxint7QG50=; b=jbzN1GU1C71vwesNnJ3H8UQcMN3d5f9inEW9EvpP47zUllR27DcF5Dy/lIZBSBVtiQ Cf+Zq4ZUKAAfC/sEgWyNmcgmgfHc5oYF3NKMYdf+1sIGAKbRVhqgbvaOP4CJrYkoUIcK 7HAPjMNZ/VwlBFGkLNyQ64nqEyyMQGfbmajmwKWdlFEAtM525xGuhfotSbTqnUtpFIou GXubYcEpNzWvXxPVaUtsn5SfBWwhprQa3wc/00dwisjFMt5GoIzb9ZoK3W8Vqih20tE7 +j85K+M4GsPAErSqc3iw8Cuy5hH7iUsz/Ip4NaZYL9LRNle/t4a4YS6SHh5YqYBYFpRX 5pHA== X-Gm-Message-State: AEkoouthpNFch7BYEJSmQMcIvRPx/mJQl9qNtagwFNrIYYQLNBanlBrJqLTvd+OC2zqvW/V0v26+J2Qkkgf+zv4N X-Received: by 10.55.122.65 with SMTP id v62mr5148492qkc.120.1470302831129; Thu, 04 Aug 2016 02:27:11 -0700 (PDT) MIME-Version: 1.0 Received: by 10.200.41.90 with HTTP; Thu, 4 Aug 2016 02:27:09 -0700 (PDT) In-Reply-To: References: From: Navnath Gadakh Date: Thu, 4 Aug 2016 14:57:09 +0530 Message-ID: Subject: Re: pgAdmin IV : Unittest modular patch To: Dave Page Cc: pgadmin-hackers , Kanchan Mohitey Content-Type: multipart/alternative; boundary=94eb2c05e0ecf64d6605393b8d61 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 --94eb2c05e0ecf64d6605393b8d61 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Hi Dave, On Wed, Aug 3, 2016 at 8:32 PM, Dave Page wrote: > On Wed, Aug 3, 2016 at 2:01 PM, Navnath Gadakh > wrote: > > 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: serv= er > >> > 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 th= e > >> > =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. > > Oh - does the per-server config override the main config? That's > useful. So anything that's in test_advanced_config.py can be > overridden on a per-server basis in test_config.py? No. per-server i.e advance config(test_advanced_config.json.in) and main config(test_config.json) both are different files. In main config we just mention the server=E2=80=99s credentials.(We can also mention per server credentails) and in test_advanced_config.json.in(here we say per-server config) we mention the advanced configurations i.e. test data for each node. > > >> - 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. > > Please output a notice saying the test was skipped, and note it in the > summary as well (as we're planning to do with failed tests). > > >> > >> - 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. > > Something like that yes - though service_account seems pointless given > your comment below... > > >> - 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 norm= al > > user, the user should be 'root' > > Say, I am logged in my machine as a 'abc' user & I have installe= d > > PostgreSQL9.5 with 'postgres' user. > > It won't run chown postgres > > /$tablespace_path_for_PostgreSQL9.5/ > > Aww crap. Yeah :-(. OK in that case let's have the user specify (and > create) the path themselves for each server. If they don't specify a > path, then skip the test. > > In fact - you might as well only skip the test in that case, and > forget about the address check above. > > So, let's summarize the discussion: - Let user specify the tablespace path in test_advanced_config.json.in - If path not specified in the test_advanced_config.json.in skip the test cases for tablespace. - Output a notice saying the test was skipped, and note it in the summary. - No need to check the address.(Unix domain socket). It seems correct way. Please tell me if I missed anything. Thanks! -- > 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 --94eb2c05e0ecf64d6605393b8d61 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hi Dave,

On Wed, Aug 3, 2016 at 8:32 PM, Dave Page <dave.page@ente= rprisedb.com> wrote:
On Wed, Aug 3, 2016 = at 2:01 PM, Navnath Gadakh
<navnath.gadakh@enterprisedb.com> wrote:
> Hi Dave,
>=C2=A0 =C2=A0 =C2=A0 Thanks for clarification.
>
> On Wed, Aug 3, 2016 at 2:45 PM, Dave Page <dave.page@enterprisedb.com>
> wrote:
>>
>> Hi Navnath
>>
>> On Tue, Aug 2, 2016 at 3:58 PM, Navnath Gadakh
>> <navnath.gad= akh@enterprisedb.com> wrote:
>> > Hi Dave,
>> >=C2=A0 =C2=A0 =C2=A0 Please find the attached patch.
>> > This patch includes:
>> > 1. API test cases for Roles & Tablespaces node(Completed = nodes: server
>> > groups, servers, databases)
>> >=C2=A0 =C2=A0 =C2=A0You can run test-suite using following com= mand
>> >=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= regression/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= regression/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= regression/runtests.py
>> >=C2=A0 =C2=A0 =C2=A0 You can also test with multiple=C2=A0 ser= vers.
>> > 2. Delete database code in some of the missed test files.
>> > 3. Added advanced configurations in test_advanced_c= onfig.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
>> >=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 gett= ing =E2=80=98UnicodeDecodeError=E2=80=99 when I run
>> > test-suite(runtests.py) with existing code. I already explain= ed the
>> > detail
>> > about this error in RM(#1521). I am creating test user to tes= t the
>> > =E2=80=98valid
>> > password=E2=80=99 test case.
>>
>> The tablespace test is one that I think is going to cause us probl= ems
>> - we really can't have a hard-coded path in the config;
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0Understood.
>>
>>
>> - It might need to be different for each server being tested
>>=C2=A0 =C2=A0Current patch supports multiple servers if you add tab= lespace
>> configurations for multiple servers. In the current patch, it'= s only for one
>> server.
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0For multiple servers you need to add multipl= e tablespace
> configuration:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"test_tablespc_credential= s":[{
>=C2=A0 =C2=A0 =C2=A0"test_tblspace_name": "test_tablespa= ce",
>=C2=A0 =C2=A0 =C2=A0"test_spc_location": "/Library/Postg= reSQL/9.6/data",
>=C2=A0 =C2=A0 =C2=A0"test_spc_opts": [],
>=C2=A0 =C2=A0 =C2=A0"test_spc_user": "XXXXX"
>=C2=A0 =C2=A0},
>=C2=A0 =C2=A0"test_tblspace_name": "test_tablespace1&quo= t;,
>=C2=A0 =C2=A0 =C2=A0"test_spc_seclable": [],
>=C2=A0 =C2=A0 =C2=A0"test_spc_location": "/Library/Postg= rePlus/9.4AS/data",
>=C2=A0 =C2=A0 =C2=A0"test_spc_opts": [],
>=C2=A0 =C2=A0 =C2=A0"test_spc_user": "YYYYY"
>=C2=A0 =C2=A0}]
>=C2=A0 =C2=A0 Anyhow, we are not going to use a hard coded path.

Oh - does the per-server config override the main config? That&= #39;s
useful. So anything that's in test_advanced_config.py can be
overridden on a per-server basis in test_config.py?
=C2=A0= =C2=A0 No.
=C2=A0 =C2=A0 per-server i.e advance config(test_advanced_config.json.in) = and main config(test_config.json) both are different files. In main config = we just =C2=A0 =C2=A0 mention the server=E2=80=99s credentials.(We can also= mention per server credentails) and in test_advanced_config.json.in(here we say per-server config= )
we mention the advanced configurations i.e. test data for each node.= =C2=A0
=C2=A0

>> - 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<= br> >> socket (hostname starts with /), 127.0.0.1 or ::1.
>
>=C2=A0 =C2=A0 =C2=A0Understood.

Please output a notice saying the test was skipped, and note it in t= he
summary as well (as we're planning to do with failed tests).

>>
>> - 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 e= db
>> for PPAS.
>
>=C2=A0 =C2=A0 =C2=A0Ok. For server connected via Unix domain socket. As= per my understanding
> modified configuration should be,
>=C2=A0 =C2=A0 =C2=A0 "tablespc_credentials":[{
>=C2=A0 =C2=A0 =C2=A0"tblspace_name": "tablespace1",=
>=C2=A0 =C2=A0 =C2=A0"testspc_seclable": [],
>=C2=A0 =C2=A0 =C2=A0"spc_acl": [
>=C2=A0 =C2=A0 =C2=A0 =C2=A0{
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"grantee":"postgres&qu= ot;,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"grantor":"postgres&qu= ot;,
>=C2=A0 =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 =C2=A0 =C2=A0"privilege_type&qu= ot;:"C",
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"privilege":t= rue,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"with_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 =C2=A0 =C2=A0],
>=C2=A0 =C2=A0 =C2=A0"spc_location": "/tmp",
>=C2=A0 =C2=A0 =C2=A0"spc_opts": [],
>=C2=A0 =C2=A0 =C2=A0"spc_user": "postgres",
>=C2=A0 =C2=A0 =C2=A0"service_account": "test1_user"=
>=C2=A0 =C2=A0},{
>=C2=A0 =C2=A0 =C2=A0"tblspace_name": "tablespace2",=
>=C2=A0 =C2=A0 =C2=A0"spc_seclable": [],
>=C2=A0 =C2=A0 =C2=A0"spc_acl": [
>=C2=A0 =C2=A0 =C2=A0 =C2=A0{
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"grantee":"enterprised= b",
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"grantor":"enterprised= b",
>=C2=A0 =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 =C2=A0 =C2=A0"privilege_type&qu= ot;:"C",
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"privilege":t= rue,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"with_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 =C2=A0 =C2=A0],
>=C2=A0 =C2=A0 =C2=A0"spc_location": "/tmp",
>=C2=A0 =C2=A0 =C2=A0"spc_opts": [],
>=C2=A0 =C2=A0 =C2=A0"spc_user": "enterprisedb",
>=C2=A0 =C2=A0 =C2=A0"service_account": "test2_user"=
>=C2=A0 =C2=A0}]
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Please correct me if I am wrong.

Something like that yes - though service_account seems pointles= s given
your comment below...

>> - Otherwise; assume the server is on the local machine, and:
>>=C2=A0 =C2=A0- Create /$tblspace_path/<random_string>/
>>=C2=A0 =C2=A0- chown $service_account /$tblspace_path/<random_st= ring>/
>>=C2=A0 =C2=A0- Run the tests
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0We can't set the owner for a tablespace = path directory using normal
> user, the user should be 'root'
>=C2=A0 =C2=A0 =C2=A0 =C2=A0Say, I am logged in my machine as a 'abc= ' user=C2=A0 & I have installed
> PostgreSQL9.5 with 'postgres' user.
>=C2=A0 =C2=A0 =C2=A0 =C2=A0It won't run chown postgres
> /$tablespace_path_for_PostgreSQL9.5/<random_string>

Aww crap. Yeah :-(. OK in that case let's have the user specify = (and
create) the path themselves for each server. If they don't specify a path, then skip the test.

In fact - you might as well only skip the test in that case, and
forget about the address check above.

=C2=A0 = =C2=A0 So, let's summarize=C2=A0the discussion:
=C2=A0 =C2=A0= - Let user specify the tablespace path in test_advanced_config.json.in
=C2=A0 =C2=A0 - = If path not specified in the test_advanced_config.json.in skip the test cases for tablespace.
=C2=A0 =C2=A0 - Output a notice saying the test was skipped, and note= it in the summary.
=C2=A0 =C2=A0 -=C2=A0No need= to check the address.(Unix domain socket). = It seems correct way.
=C2=A0 =C2=A0
=C2=A0 =C2=A0Please= tell me if I missed anything.
=C2=A0=C2=A0
=C2=A0 =C2= =A0Thanks! =C2=A0 =C2=A0=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>

=
--94eb2c05e0ecf64d6605393b8d61--