Hi Akshay I really appreciate you taking a look at this on a Sunday! I am not able to get access to my work computer to run your changes directly. From what you are saying, though, it sounds like a reasonable change. If this change is passing in your environment, I say go ahead and make the change to the patch and commit.
Thanks again for helping us get this into the release. Best, Matt On Sun, May 14, 2017 at 4:52 AM, Akshay Joshi <akshay.jo...@enterprisedb.com > wrote: > Hi Joao > > On Fri, May 12, 2017 at 11:23 PM, Joao Pedro De Almeida Pereira < > jdealmeidapere...@pivotal.io> wrote: > >> 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 >> > > Test case still failing on my machine because when we create new Role > it is without password and when we try to create table with that role first > we will try to connect the database using that new role with > servers['db_password'] which is wrong. I have modified setup function of " > *test_role_dependencies_sql.py*" and it works, we can create new role > using database server's password > > Old setup function: > *def *setUp(self): > > create_role(self.server, *"testpgadmin"*) > self.server_with_modified_user = self.server.copy() > self.server_with_modified_user[*'username'*] = *"testpgadmin"* > > > Modified setup function: > > > def setUp(self): > > with test_utils.Database(self.server) as (connection, database_name): > cursor = connection.cursor() > try: > cursor.execute( > "CREATE ROLE testpgadmin LOGIN PASSWORD '%s'" > % self.server['db_password']) > except Exception as exception: > print(exception) > connection.commit() > > self.server_with_modified_user = self.server.copy() > self.server_with_modified_user['username'] = "testpgadmin" > > > If the above logic looks good to you then I'll commit the code. > >> >> Thanks >> Joao & George >> >> On Thu, May 11, 2017 at 7:33 AM, Joao Pedro De Almeida Pereira < >> jdealmeidapere...@pivotal.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.va...@enterprisedb.com> wrote: >>> >>>> On Thu, May 11, 2017 at 12:03 PM, Dave Page <dp...@pgadmin.org> wrote: >>>> >>>>> >>>>> >>>>> On 11 May 2017, at 07:11, Akshay Joshi <akshay.jo...@enterprisedb.com> >>>>> wrote: >>>>> >>>>> Hi Sarah >>>>> >>>>> On Thu, May 11, 2017 at 2:30 AM, Sarah McAlear <smcal...@pivotal.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.jo...@enterprisedb.com> wrote: >>>>>> >>>>>>> >>>>>>> >>>>>>> On May 10, 2017 19:07, "Sarah McAlear" <smcal...@pivotal.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.jo...@enterprisedb.com> wrote: >>>>>>> >>>>>>>> Hi Sarah >>>>>>>> >>>>>>>> On Tue, May 9, 2017 at 9:03 PM, Sarah McAlear <smcal...@pivotal.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>* >>>>> >>>>> >> > > > -- > *Akshay Joshi* > *Principal Software Engineer * > > > > *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91 976-788-8246 > <+91%2097678%2088246>* >
