public inbox for [email protected]  
help / color / mirror / Atom feed
From: Navnath Gadakh <[email protected]>
To: Dave Page <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Cc: Kanchan Mohitey <[email protected]>
Subject: Re: pgAdmin IV : Unittest modular patch
Date: Thu, 4 Aug 2016 14:57:09 +0530
Message-ID: <CAOAJCYrzjjrf-qpGH+rm4cWajvcYuyQH8jufqWBFL3qVAVpSoA@mail.gmail.com> (raw)
In-Reply-To: <CA+OCxowBEaow-6X1NzcA_i1qzKoiXJprh33s1wciriMup3_omA@mail.gmail.com>
References: <CAOAJCYrcxBN2J_3dWq0Cwc82fkugzUi3NKD4jKoTKRBot=ev8w@mail.gmail.com>
	<CAOAJCYpSBXcfEC5d4Sv_=jV86m1=rKUK0tMkZFhdSHkPb=XT3A@mail.gmail.com>
	<CA+OCxowW0wjpNT-o0YF1Bg9rtD6Z7sqp35ZRUgoEguVKO4OtSw@mail.gmail.com>
	<CAOAJCYr1JwYK4x2wDYw2UVAZoXC5a0FQT7Yz1Y_Wn46kzPMmQg@mail.gmail.com>
	<CA+OCxoxqahNgLmvQqqErQZL2JS0EaQwyXkgkzvkFtGNh0OKV6w@mail.gmail.com>
	<CAOAJCYqQQ4MaudVaVwfcateuxum-W39HwRiEorhYP8zbqjxN0g@mail.gmail.com>
	<CA+OCxozZVCwB4t6hXjMeHbBV8Fx1cm_Cg1tknjWyYxLFZNYsEQ@mail.gmail.com>
	<CAFOhELes8Vu_VciiVu31wfaYtTuCFUReEF1mfNXB4nSMcTNrHA@mail.gmail.com>
	<CAOAJCYpY=u4T38s+XqQDOky10e7A_Wb_q9bbiwhx_=eF-6i-dQ@mail.gmail.com>
	<CA+OCxoxm9_f5PJVLhwrSk6bozmivv_-=o+spffXjkYk+qPDKeg@mail.gmail.com>
	<CAOAJCYpcye6nQAPB8os9vZXOhAf9wEvr6rA9i4vuZLDVrRYF0g@mail.gmail.com>
	<CA+OCxowE5D0N2oq-Da+eXh4+cZMA3=GqB85-nE+YHuaACfjhbQ@mail.gmail.com>
	<CAOAJCYonmDD-oqjwhJNVpWVHxyf-bTn_eJ8o7wNMr7sOOpBvkA@mail.gmail.com>
	<CA+OCxozHiy2UsJp6jvYRNfYv3RWTijdFeoPuZH4dqyyT-knnjQ@mail.gmail.com>
	<CAOAJCYo4Lyc3f+-6a7cYxsxjn9GKG7j7R8VYiDmxgrntyds-xg@mail.gmail.com>
	<CA+OCxoxKo1L9ZNH81h-kmzWvzn7Pj=T4fUnP6FpvchePpoAB0A@mail.gmail.com>
	<CAOAJCYq94E=zhaVWWUORP62nSD-PJ2y2crJeL41Mmv-50i-VbQ@mail.gmail.com>
	<CA+OCxoy2kSbeBDCYaXGKOvEZAtpEO7uviL5cPakNf0FM=C=dzg@mail.gmail.com>
	<CAOAJCYpPicjz4cOsTHHGfMRN7qgeNwdvbkyS4ihy5FZYPgUyHQ@mail.gmail.com>
	<CA+OCxowBEaow-6X1NzcA_i1qzKoiXJprh33s1wciriMup3_omA@mail.gmail.com>
List-Unsubscribe:  <mailto:[email protected]?body=unsub%20pgadmin-hackers>

Hi Dave,

On Wed, Aug 3, 2016 at 8:32 PM, Dave Page <[email protected]>
wrote:

> On Wed, Aug 3, 2016 at 2:01 PM, Navnath Gadakh
> <[email protected]> wrote:
> > Hi Dave,
> >      Thanks for clarification.
> >
> > On Wed, Aug 3, 2016 at 2:45 PM, Dave Page <[email protected]>
> > wrote:
> >>
> >> Hi Navnath
> >>
> >> On Tue, Aug 2, 2016 at 3:58 PM, Navnath Gadakh
> >> <[email protected]> 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
> >> > ‘valid
> >> > password’ 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 ‘UnicodeDecodeError’ 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
> >> > ‘valid
> >> > password’ 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’s 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/<random_string>/
> >>   - chown $service_account /$tblspace_path/<random_string>/
> >>   - 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/<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.
>
>     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
>



-- 
Thanks,
Navnath Gadakh
Software Engineer
EnterpriseDB Corporation
Mobile: +91 9975389878


view thread (29+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: pgAdmin IV : Unittest modular patch
  In-Reply-To: <CAOAJCYrzjjrf-qpGH+rm4cWajvcYuyQH8jufqWBFL3qVAVpSoA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox