Re: [patch] Dependents and Dependencies in GreenPlum

Hello Akshay,

You can find attached a new version of the patch that changes the test in
question.
This new version instead of requiring a creation of a new Role now does the
following:
1 - Creates a new Role
2 - Creates a table with that Role
3 - Checks for dependencies

Thanks
Joao & George

On Thu, May 11, 2017 at 7:33 AM, Joao Pedro De Almeida Pereira <
jdealmeidapereira(at)pivotal(dot)io> wrote:

> Hello all,
> This test is trying to check if the sql to retrieve the role dependencies
> of a object in the database is correct. Using a different user to connect
> to the database and create tables there is no need for extra setup because
> the role depedencies table entry is added automatically. Also we talked
> with some internal users and they told us they always used a different user
> to connect to the database with pgadmin.
>
> After your comments we realize that the test need to have a different
> setup in order to become more readable and resilient. What do we need to do
> in the test setup to ensure an entry is added to the role dependency table?
>
> Also the test need to be a little bit more resilient to work independently
> of the user that is used on the test.
>
> Thanks
> Joao
>
> On Thu, May 11, 2017, 3:20 AM Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)
> com> wrote:
>
>> On Thu, May 11, 2017 at 12:03 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>>
>>>
>>> On 11 May 2017, at 07:11, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
>>> wrote:
>>>
>>> Hi Sarah
>>>
>>> On Thu, May 11, 2017 at 2:30 AM, Sarah McAlear <smcalear(at)pivotal(dot)io>
>>> wrote:
>>>
>>>> Hi Ashkay!
>>>>
>>>> TestTablesNodeSql hasn't failed but TestRoleDependenciesSql failed with
>>>>> below error:
>>>>> .....\test_role_dependencies_sql.py\", line 41, in assertions\n
>>>>> self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0
>>>>
>>>>
>>>> We experienced a similar problem when we started developing this patch.
>>>> We noticed that we were connecting to the database with the super user (the
>>>> db owner user), which did not create the role we are expecting when a table
>>>> is created. To ensure that this role is created, we had to create a new
>>>> user and use it to execute all the tests. After we made this change, the
>>>> tests passed.
>>>>
>>>> Can you try to create a new user and use it to execute your tests?
>>>>
>>>
>>> I did that and it works. I have created new user 'test' with
>>> superuser privileges and update parameter db_username="test" in
>>> test_config.json file. But still I am unable to understand why it doesn't
>>> work with 'postgres' (default) user?
>>>
>>>
>>> Agreed. That suggests something fishy is going on that should be
>>> understood.
>>>
>>>
>> We don't check role dependencies for every object. As per the code, the
>> role dependencies are executed for columns not for the table itself.
>> And in the test cases, the table object is trying to execute the role
>> dependencies, so I think this test case is not correct.
>>
>>>
>>>> Thanks,
>>>> João & Sarah
>>>>
>>>> On Wed, May 10, 2017 at 1:56 PM, Akshay Joshi <
>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>>
>>>>>
>>>>> On May 10, 2017 19:07, "Sarah McAlear" <smcalear(at)pivotal(dot)io> wrote:
>>>>>
>>>>> Hi Akshay!
>>>>>
>>>>> Does this error occur on 9.6 or 10.0? We tested it in 9.6 and all our
>>>>> tests pass.
>>>>>
>>>>>
>>>>> On both 9.6 and 10.0. Query returned 0 rows which is compared
>>>>> against 1 in assert statement.
>>>>>
>>>>>
>>>>> Thanks!
>>>>> João & Sarah
>>>>>
>>>>> On Wed, May 10, 2017 at 2:29 AM, Akshay Joshi <
>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi Sarah
>>>>>>
>>>>>> On Tue, May 9, 2017 at 9:03 PM, Sarah McAlear <smcalear(at)pivotal(dot)io>
>>>>>> wrote:
>>>>>>
>>>>>>> Hi Akshay!
>>>>>>>
>>>>>>>
>>>>>>>> Some test file names ended with "*_sql_template.py*" do we need to
>>>>>>>> add that string ?
>>>>>>>
>>>>>>> we added this suffix after moving the tests up a level to
>>>>>>> tables/tests to clarify what subject they applied to. we changed the suffix
>>>>>>> to "_sql.py"
>>>>>>>
>>>>>>> - Files "test_column_acl_sql_template.py" and "
>>>>>>>> test_column_properties_sql_template.py" should be moved from
>>>>>>>> tables->tests to tables->column->tests. As it's related to column.
>>>>>>>> - Files "test_trigger_get_oid_sql_template.py" and "
>>>>>>>> test_trigger_nodes_sql_template.py" should be moved from
>>>>>>>> tables->tests to tables->triggers->tests. As its related to triggers.
>>>>>>>
>>>>>>> these tests are related to the sql files in tables/templates/column,
>>>>>>> not tables/column, so moving them into tables/column would be more
>>>>>>> confusing.
>>>>>>>
>>>>>>> Following test cases are failing
>>>>>>>
>>>>>>> Thank you, fixed, see new patch. Can you confirm that
>>>>>>> TestTablesNodeSql doesn't fail in your environment?
>>>>>>>
>>>>>>
>>>>>> TestTablesNodeSql hasn't failed but TestRoleDependenciesSql
>>>>>> failed with below error:
>>>>>> .....\test_role_dependencies_sql.py\", line 41, in assertions\n
>>>>>> self.assertEqual(1, len(fetch_result))\nAssertionError: 1 != 0
>>>>>>
>>>>>>
>>>>>>> Thanks,
>>>>>>> George & Sarah
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> *Akshay Joshi*
>>>>>> *Principal Software Engineer *
>>>>>>
>>>>>>
>>>>>>
>>>>>> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91
>>>>>> 976-788-8246 <+91%2097678%2088246>*
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>> --
>>> *Akshay Joshi*
>>> *Principal Software Engineer *
>>>
>>>
>>>
>>> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91
>>> 976-788-8246 <+91%2097678%2088246>*
>>>
>>>