Re: pgAdmin IV : Unittest modular patch

Hi Dave,
Thanks for clarification.

On Wed, Aug 3, 2016 at 2:45 PM, Dave Page <dave(dot)page(at)enterprisedb(dot)com>
wrote:

> Hi Navnath
>
> On Tue, Aug 2, 2016 at 3:58 PM, Navnath Gadakh
> <navnath(dot)gadakh(at)enterprisedb(dot)com> 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.

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

>
> 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 = json.load(data_file)
> except:
> with open(CURRENT_PATH + '/test_advanced_config.json.in') as
> data_file:
> advanced_config_data = 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
>

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