Derby
  1. Derby
  2. DERBY-2410

Convert grantRevoke.java to JUnit

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.3.1.4
    • Fix Version/s: 10.3.1.4
    • Component/s: Test
    • Labels:
      None

      Description

      Convert grantRevoke.java to JUnit. Also add test cases from grantRevokeDDL and grantRevokeDDL2 SQL tests to the new GrantRevokeTest

      1. derby-2410-pre.diff
        29 kB
        Andrew McIntyre
      2. derby-2410-pre2.diff
        48 kB
        Andrew McIntyre
      3. derby-2410-pre3.diff
        53 kB
        Andrew McIntyre
      4. derby-2410-pre4.diff
        55 kB
        Andrew McIntyre
      5. derby-2410-pre5.diff
        59 kB
        Andrew McIntyre
      6. derby-2410-v1.diff
        61 kB
        Andrew McIntyre

        Issue Links

          Activity

          Hide
          Andrew McIntyre added a comment -

          Attaching an early draft for converting grantRevoke.java to JUnit. I'd like to get some feedback on the approach, and some extra eyes to make sure I'm not making any mistakes in the logic. This draft does implement all of the testcases from the grant part of grantRevoke.java, although not all of the checks have been implemented yet, as I've encountered some problems.

          • assertInsertPrivilege and assertUpdatePrivilege need to be completed. Currently having an issue with getColumns returning an empty ResultSet for tables when the DatabaseMetaData is acquired from the connection opened with openUserConnection
          • problems with the triggers in testGrantRollbackAndCommit need to be investigated.
          • it might be a good idea to have a version of the assert methods, or at least for select, that takes an array of users, so public privileges can be asserted against all users.
          • assertReferences needs an implementation, although it should be noted that the previous version of the test did not have an implementation for references checks.

          Questions:

          • Is client/server necessary here? I guess verifying the metadata methods work properly is worth running on the client, although perhaps the client/server tests should restrict themselves to the metadata test fixtures.
          • adding the revoke testcases and the testcases from grantrevokeDDL and grantrevokeDDL2 will make this a large test. Any opinion on whether or not to split these tests up into several test classes? And what approach: move utility methods into BaseJDBCTestCase? Have a GrantRevokeTestCase that the individual tests extend?

          Left to do from grantrevoke.java:

          • revoke testcases
          • standard error testcases
          Show
          Andrew McIntyre added a comment - Attaching an early draft for converting grantRevoke.java to JUnit. I'd like to get some feedback on the approach, and some extra eyes to make sure I'm not making any mistakes in the logic. This draft does implement all of the testcases from the grant part of grantRevoke.java, although not all of the checks have been implemented yet, as I've encountered some problems. assertInsertPrivilege and assertUpdatePrivilege need to be completed. Currently having an issue with getColumns returning an empty ResultSet for tables when the DatabaseMetaData is acquired from the connection opened with openUserConnection problems with the triggers in testGrantRollbackAndCommit need to be investigated. it might be a good idea to have a version of the assert methods, or at least for select, that takes an array of users, so public privileges can be asserted against all users. assertReferences needs an implementation, although it should be noted that the previous version of the test did not have an implementation for references checks. Questions: Is client/server necessary here? I guess verifying the metadata methods work properly is worth running on the client, although perhaps the client/server tests should restrict themselves to the metadata test fixtures. adding the revoke testcases and the testcases from grantrevokeDDL and grantrevokeDDL2 will make this a large test. Any opinion on whether or not to split these tests up into several test classes? And what approach: move utility methods into BaseJDBCTestCase? Have a GrantRevokeTestCase that the individual tests extend? Left to do from grantrevoke.java: revoke testcases standard error testcases
          Hide
          Andrew McIntyre added a comment -

          Attaching second early draft of the conversion for this test. All the testcases from grantRevoke.java have now been implemented, and all testcases in this patch pass, but there remain several issues to resolve:

          • assertInsertPrivilege and assertUpdatePrivilege need to be implemented. Currently having a problem with DatabaseMetaData.getColumns for the user connections returning an empty ResultSet. This is the next problem I will try to resolve.
          • I think some reworking of the rollback tests is in order. While the tests currently pass as expected, I think that a variant of grant() is needed that takes a connection so that we are certain the grants occur on the owner connection, so we can be certain that the rollback and commit are doing what we expect.
          • assertTriggerPrivilege has an issue in the rollback tests where an error that the trigger already exists occurs. This needs investigation and may be an actual bug.
          • variant of assertSelectPrivilege that takes an array of users to verify public privileges, as mentioned in previous comment
          • review grantRevoke.java and make sure that no functionality tested there is missing from the new test

          after the above have been resolved:

          • move testcases over from grantRevokeDDL / grantRevokeDDL2.
          • assertReferencesPrivilege implementation.
          Show
          Andrew McIntyre added a comment - Attaching second early draft of the conversion for this test. All the testcases from grantRevoke.java have now been implemented, and all testcases in this patch pass, but there remain several issues to resolve: assertInsertPrivilege and assertUpdatePrivilege need to be implemented. Currently having a problem with DatabaseMetaData.getColumns for the user connections returning an empty ResultSet. This is the next problem I will try to resolve. I think some reworking of the rollback tests is in order. While the tests currently pass as expected, I think that a variant of grant() is needed that takes a connection so that we are certain the grants occur on the owner connection, so we can be certain that the rollback and commit are doing what we expect. assertTriggerPrivilege has an issue in the rollback tests where an error that the trigger already exists occurs. This needs investigation and may be an actual bug. variant of assertSelectPrivilege that takes an array of users to verify public privileges, as mentioned in previous comment review grantRevoke.java and make sure that no functionality tested there is missing from the new test after the above have been resolved: move testcases over from grantRevokeDDL / grantRevokeDDL2. assertReferencesPrivilege implementation.
          Hide
          Andrew McIntyre added a comment -

          Attaching third draft for converting this test. Insert and update privileges now have proper assert methods. DatabaseMetaData.getColumns() was failing if the schema / table names were not uppercased, even though they were not quoted. I will follow up at a later date to see if this is or is not expected behavior.

          Current issues:

          • assertTriggerPrivilege fails in some tests with complaints that the trigger already exists. This is either a failure by assertTriggerPrivilege to drop a trigger once created, or a bug. Probably the former.
          • Attempted to have the rollback tests execute their grant statements explicitly on a connection owned by the database owner. This led to lock timeouts, however, so I've reverted to the previous method which simply executes the statements on the default connection via runSQLCommands() for now.

          remaining todo items:

          • move testcases over from grantrevokeDDL / grantrevokeDDL2
          • implement checks on privilege metadata in system catalogs
          • test DatabaseMetaData methods getColumnPrivileges / getTablePrivileges
          • variants of assertXXXPrivileges that takes an array of users to assert public privileges
          • assertReferencesPrivilege implementation
          • review grantrevoke tests this test replaces to verify all expected functionality is represented
          • followup on DatabaseMetaData.getColumns() issue
          Show
          Andrew McIntyre added a comment - Attaching third draft for converting this test. Insert and update privileges now have proper assert methods. DatabaseMetaData.getColumns() was failing if the schema / table names were not uppercased, even though they were not quoted. I will follow up at a later date to see if this is or is not expected behavior. Current issues: assertTriggerPrivilege fails in some tests with complaints that the trigger already exists. This is either a failure by assertTriggerPrivilege to drop a trigger once created, or a bug. Probably the former. Attempted to have the rollback tests execute their grant statements explicitly on a connection owned by the database owner. This led to lock timeouts, however, so I've reverted to the previous method which simply executes the statements on the default connection via runSQLCommands() for now. remaining todo items: move testcases over from grantrevokeDDL / grantrevokeDDL2 implement checks on privilege metadata in system catalogs test DatabaseMetaData methods getColumnPrivileges / getTablePrivileges variants of assertXXXPrivileges that takes an array of users to assert public privileges assertReferencesPrivilege implementation review grantrevoke tests this test replaces to verify all expected functionality is represented followup on DatabaseMetaData.getColumns() issue
          Hide
          Andrew McIntyre added a comment -

          When asserting that trigger privilege was not available, the trigger that was created was not properly dropped. AssertTriggerPrivilege now rolls back the transaction, so the db returns to its previous state regardless of whether the trigger create succeeds or fails.

          Problems with lock timeouts in the rollback tests continue, added a testcase from grantRevokeDDL.sql.

          Show
          Andrew McIntyre added a comment - When asserting that trigger privilege was not available, the trigger that was created was not properly dropped. AssertTriggerPrivilege now rolls back the transaction, so the db returns to its previous state regardless of whether the trigger create succeeds or fails. Problems with lock timeouts in the rollback tests continue, added a testcase from grantRevokeDDL.sql.
          Hide
          Andrew McIntyre added a comment -

          Did a quick analysis of the lock timeout problem I was seeing with the grant rollback test (GrantRevokeTest.testGrantRollbackAndCommit()):

          I set derby.locks.waitTimeout=-1 and ran the test in client/server mode so that I could connect and dump the lock table. The lock table shows me this for SYSTABLEPERMS:

          697 |ROW |X |SYSTABLEPERMS |(1,12) |GRANT|S |4 |NULL
          700 |ROW |S |SYSTABLEPERMS |(1,12) |WAIT |S |0 |NULL

          It appears that the original transaction (697), which is the connection by the database owner which issued the grant statement, has acquired an exclusive lock on row 12 to update SYSTABLEPERMS for the added permission. From reading the code, I believe this is expected, as the grant statement is always executed with isolation level REPEATABLE_READ. (See DataDictionaryImpl.addRemovePermissionsDescriptor():10857 ). When assertSelectPrivilege() in the test code tries to do a select on the table in question, the code that checks permissions for the table for the select statement blocks waiting to read the row in SYSTABLEPERMS for which the original transaction has acquired the exclusive lock to update the permissions, which we see in the lock table dump as the WAIT for the same row (1,12).

          From reading the code, this would appear to be expected behavior, yet the old grantRevoke.java test doesn't seem to hit this problem, and from reading the old test code, it appears to be doing almost exactly what my test code is doing. So, I'm a bit stumped at the moment as to what is going on that is causing my test to block, while the old test continues on with its processing.

          I'm sure I've missed something subtle in the locking behavior, here. Or, I suppose, I've misread the old test and somewhere along the way the grant statement was being committed before the select checks were occurring, in which case I think the old test wasn't actually testing what was expected to be tested.

          So, if anyone has any insight as to why I'm hitting these lock timeouts, I would greatly appreciate it.

          Show
          Andrew McIntyre added a comment - Did a quick analysis of the lock timeout problem I was seeing with the grant rollback test (GrantRevokeTest.testGrantRollbackAndCommit()): I set derby.locks.waitTimeout=-1 and ran the test in client/server mode so that I could connect and dump the lock table. The lock table shows me this for SYSTABLEPERMS: 697 |ROW |X |SYSTABLEPERMS |(1,12) |GRANT|S |4 |NULL 700 |ROW |S |SYSTABLEPERMS |(1,12) |WAIT |S |0 |NULL It appears that the original transaction (697), which is the connection by the database owner which issued the grant statement, has acquired an exclusive lock on row 12 to update SYSTABLEPERMS for the added permission. From reading the code, I believe this is expected, as the grant statement is always executed with isolation level REPEATABLE_READ. (See DataDictionaryImpl.addRemovePermissionsDescriptor():10857 ). When assertSelectPrivilege() in the test code tries to do a select on the table in question, the code that checks permissions for the table for the select statement blocks waiting to read the row in SYSTABLEPERMS for which the original transaction has acquired the exclusive lock to update the permissions, which we see in the lock table dump as the WAIT for the same row (1,12). From reading the code, this would appear to be expected behavior, yet the old grantRevoke.java test doesn't seem to hit this problem, and from reading the old test code, it appears to be doing almost exactly what my test code is doing. So, I'm a bit stumped at the moment as to what is going on that is causing my test to block, while the old test continues on with its processing. I'm sure I've missed something subtle in the locking behavior, here. Or, I suppose, I've misread the old test and somewhere along the way the grant statement was being committed before the select checks were occurring, in which case I think the old test wasn't actually testing what was expected to be tested. So, if anyone has any insight as to why I'm hitting these lock timeouts, I would greatly appreciate it.
          Hide
          Andrew McIntyre added a comment -

          This will be the last of my 'pre' patches for this issue. I've converted all of the testcases in grantRevoke.java, but there remain several issues to resolve / doc:

          • DatabaseMetaData methods seem to currently require uppercase schema / table names. This seems to be expected behavior based on my reading of the code and metadata queries, although I could not find any documentation for it.
          • privileges granted by the database owner in an open transaction block user transactions for the same column/table because grant/revoke statements are always executed with repeatable read isolation level. Again, I believe this is expected behavior from reading the code, but could not find any documentation for it.
          • one testcase checking update privilege on all columns of a table for a user fails to get table privilege for that user using DatabaseMetaData.getTablePrivileges(), worth investigating.
          • one testcase checking insert privilege cannot be verified via DatabaseMetaData for all columns of a table after rollback of a transaction including grant/revoke statements, needs further investigation.
          • need check for REFERENCES privilege missing from previous version of the test
          • trigger privilege is asserted by creating the trigger with autoCommit(false) and then rolling the transaction back, but attempting to assert that the trigger privilege has been granted by checking the DatabaseMetaData, while the transaction being used to create the trigger to assert the privilege is active, causes a lock timeout. Probably related to grant / revoke statements always being executed with isolation level RR.
          • checking DatabaseMetaData.getColumnPrivileges() for every column in a table, when attempting to assert that we have permission on every column in that table fails. I highly suspect that my logic in checking the permissions is incorrect and would really like another set of eyes here. See the comments in the test for assertPrivilegeMetaData().

          I still have several tests I plan to run to check the behavior of the previous grantRevoke.java test against my conversion, but because it is not a junit test, it is slow going. Assuming there is no feedback on the issues above, I will commit the attached test in a few days and remove the old grantRevoke.java.

          Once / If the above are resolved / committed, I plan to convert additional testcases from grantRevokeDDL.sql and grantRevokeDDL2.sql to complete this issue.

          Show
          Andrew McIntyre added a comment - This will be the last of my 'pre' patches for this issue. I've converted all of the testcases in grantRevoke.java, but there remain several issues to resolve / doc: DatabaseMetaData methods seem to currently require uppercase schema / table names. This seems to be expected behavior based on my reading of the code and metadata queries, although I could not find any documentation for it. privileges granted by the database owner in an open transaction block user transactions for the same column/table because grant/revoke statements are always executed with repeatable read isolation level. Again, I believe this is expected behavior from reading the code, but could not find any documentation for it. one testcase checking update privilege on all columns of a table for a user fails to get table privilege for that user using DatabaseMetaData.getTablePrivileges(), worth investigating. one testcase checking insert privilege cannot be verified via DatabaseMetaData for all columns of a table after rollback of a transaction including grant/revoke statements, needs further investigation. need check for REFERENCES privilege missing from previous version of the test trigger privilege is asserted by creating the trigger with autoCommit(false) and then rolling the transaction back, but attempting to assert that the trigger privilege has been granted by checking the DatabaseMetaData, while the transaction being used to create the trigger to assert the privilege is active, causes a lock timeout. Probably related to grant / revoke statements always being executed with isolation level RR. checking DatabaseMetaData.getColumnPrivileges() for every column in a table, when attempting to assert that we have permission on every column in that table fails. I highly suspect that my logic in checking the permissions is incorrect and would really like another set of eyes here. See the comments in the test for assertPrivilegeMetaData(). I still have several tests I plan to run to check the behavior of the previous grantRevoke.java test against my conversion, but because it is not a junit test, it is slow going. Assuming there is no feedback on the issues above, I will commit the attached test in a few days and remove the old grantRevoke.java. Once / If the above are resolved / committed, I plan to convert additional testcases from grantRevokeDDL.sql and grantRevokeDDL2.sql to complete this issue.
          Hide
          Andrew McIntyre added a comment -

          Attaching first complete patch for this issue. The new test covers all functionality covered by grantRevoke.java minus the column privilege checking that requires the use of FormatableBitSet, which is an internal API that the tests should not be using.

          Three issues were encountered during the conversion of this test. They all seem to be expected, but undocumented, behavior:

          • DatabaseMetaData methods require that the schema and tablename be uppercased. Passing lowercase names to the create / grant / revoke statements work as expected, but then using the same lowercase names with DatabaseMetaData returns an empty ResulSet for the metadata. This is obvious when looking at the metadata queries and how the metadata is returned to the user. The lack of documentation of this behavior surprised me more than anything.
          • as mentioned in a previous mail, privileges granted by the database owner in an open transaction block user transactions for the same column/table because grant/revoke statements are always executed with repeatable read isolation level. From reading the code, this is expected, but doesn't appear to be documented anywhere. I've been meaning to double-check this behavior with the old test (which, on first reading, seems to allow concurrent read of grant permissions by owner and user transactions), but haven't had the time to get the test running in the debugger with the old harness to check.
          • Table level privileges are not recorded in SYSCOLPERMS and column level privileges where the set of columns is equivalent to the set of columns in the table is not recorded in SYSTABLEPERMS. Again, I believe this to be expected - but not obvious - behavior.

          other comments:

          • I feel that a true universal test for references privilege is impossible, since it requires that the referenced column be a primary key or unique constraint. i.e. it is possible to grant and have references privilege on a column for which it has no meaning to have that privilege. Therefore, no meaningful universal test for that privilege can be crafted. That said, I left an untested implementation for an assert of the privilege in assertReferencesPrivilege() for future use, for cases where we know the table has a primary key that can be referenced, or a unique constraint that can be referenced.
          • Some RESOLVE comments from the previous version of the test were preserved, primarily in the decorateSQL method, concerning support for grant / revoke with certain database features.

          If there are no comments, I'll commit this tomorrow afternoon and remove grantRevoke.java. Converting grantRevokeDDL.java and grantRevokeDDL2.java completely to JUnit may be more trouble than it is worth in the long run. I'm looking into converting the 15 to 16 testcases that cause differences between platforms into GrantRevokeTest and then leaving the rest of the old grantRevokeDDL tests to run under LangScriptTests.

          Show
          Andrew McIntyre added a comment - Attaching first complete patch for this issue. The new test covers all functionality covered by grantRevoke.java minus the column privilege checking that requires the use of FormatableBitSet, which is an internal API that the tests should not be using. Three issues were encountered during the conversion of this test. They all seem to be expected, but undocumented, behavior: DatabaseMetaData methods require that the schema and tablename be uppercased. Passing lowercase names to the create / grant / revoke statements work as expected, but then using the same lowercase names with DatabaseMetaData returns an empty ResulSet for the metadata. This is obvious when looking at the metadata queries and how the metadata is returned to the user. The lack of documentation of this behavior surprised me more than anything. as mentioned in a previous mail, privileges granted by the database owner in an open transaction block user transactions for the same column/table because grant/revoke statements are always executed with repeatable read isolation level. From reading the code, this is expected, but doesn't appear to be documented anywhere. I've been meaning to double-check this behavior with the old test (which, on first reading, seems to allow concurrent read of grant permissions by owner and user transactions), but haven't had the time to get the test running in the debugger with the old harness to check. Table level privileges are not recorded in SYSCOLPERMS and column level privileges where the set of columns is equivalent to the set of columns in the table is not recorded in SYSTABLEPERMS. Again, I believe this to be expected - but not obvious - behavior. other comments: I feel that a true universal test for references privilege is impossible, since it requires that the referenced column be a primary key or unique constraint. i.e. it is possible to grant and have references privilege on a column for which it has no meaning to have that privilege. Therefore, no meaningful universal test for that privilege can be crafted. That said, I left an untested implementation for an assert of the privilege in assertReferencesPrivilege() for future use, for cases where we know the table has a primary key that can be referenced, or a unique constraint that can be referenced. Some RESOLVE comments from the previous version of the test were preserved, primarily in the decorateSQL method, concerning support for grant / revoke with certain database features. If there are no comments, I'll commit this tomorrow afternoon and remove grantRevoke.java. Converting grantRevokeDDL.java and grantRevokeDDL2.java completely to JUnit may be more trouble than it is worth in the long run. I'm looking into converting the 15 to 16 testcases that cause differences between platforms into GrantRevokeTest and then leaving the rest of the old grantRevokeDDL tests to run under LangScriptTests.
          Hide
          Daniel John Debrunner added a comment -

          andrew> DatabaseMetaData methods require that the schema and tablename be uppercased. ... The lack of documentation of this behavior surprised me more than anything.

          This is documented in the javadoc for DatabaseMetaData , e.g. "tableNamePattern - a table name pattern; must match the table name as it is stored in the database"
          If DERBY-2092 was addressed then it would be part of Derby's documentation set.

          andrew> - I feel that a true universal test for references privilege is impossible, since ...

          I didn't understand this paragraph, not sure what a "universal test" is, and this comment lost me "grant and have references privilege on a column for which it has no meaning to have that privilege"

          Show
          Daniel John Debrunner added a comment - andrew> DatabaseMetaData methods require that the schema and tablename be uppercased. ... The lack of documentation of this behavior surprised me more than anything. This is documented in the javadoc for DatabaseMetaData , e.g. "tableNamePattern - a table name pattern; must match the table name as it is stored in the database" If DERBY-2092 was addressed then it would be part of Derby's documentation set. andrew> - I feel that a true universal test for references privilege is impossible, since ... I didn't understand this paragraph, not sure what a "universal test" is, and this comment lost me "grant and have references privilege on a column for which it has no meaning to have that privilege"
          Hide
          Andrew McIntyre added a comment -

          dan>This is documented in the javadoc for DatabaseMetaData , e.g. "tableNamePattern - a table name pattern; must match the table name as it is stored in the database" If DERBY-2092 was addressed then it would be part of Derby's documentation set.

          I was surprised that there was no mention of this implementation detail anywhere in the reference guide, i.e. the reference guide never mentions what is 'the table name as it is stored in the database' as far as I can tell. The actual behavior itself was not surprising, just ran into it after passing around schema / tablenames as Strings and ending up with empty ResultSets for some DatabaseMetaData methods in some cases, so I checked the docs and did not find it mentioned there.

          dan> I didn't understand this paragraph, not sure what a "universal test" is, and this comment lost me "grant and have references privilege on a column for which it has no meaning to have that privilege"

          Sorry, it was late and I could have been more clear here. It is possible to grant references privilege on any column, not just primary keys or unique constraints. You can grant references on any column in the database, and Derby happily records that privilege, and you can see in the test that this privilege on the column can be confirmed from the metadata.

          I checked the SQL standard, grant for references seems to be allowed to be granted on arbitrary tables / columns in case those table / columns in the grant are at some point modified to have / be a primary key or have a unique constraint placed on them. However, having the privilege doesn't mean you can do anything useful with it without a primary key or unique constraint on the table / column, and there's no way to check the use of it without modifying the original table. i.e. in the style of the test:

          "create table foo (c1 int not null primary key, c2)"
          grant("references"..."c2");
          assertReferencePrivilege("true"..."c2");

          passes if you just check the metadata for the privilege, but you can't test in the assert method that you have references privilege on c2 without modifying table foo in the assert method. I feel this sort of violates the spirit of the assert tests, in that they shouldn't be changing the state of the things being asserted, just verifying their state. It's probably also why this check in the previous test was left stubbed out, since there's no good check for it that doesn't alter the original table's definition.

          It's not a problem, just making a note of it for the next person who comes along and wonders what is going on here. I'll go back and double-check the comments in the test to make sure they are clear.

          Show
          Andrew McIntyre added a comment - dan>This is documented in the javadoc for DatabaseMetaData , e.g. "tableNamePattern - a table name pattern; must match the table name as it is stored in the database" If DERBY-2092 was addressed then it would be part of Derby's documentation set. I was surprised that there was no mention of this implementation detail anywhere in the reference guide, i.e. the reference guide never mentions what is 'the table name as it is stored in the database' as far as I can tell. The actual behavior itself was not surprising, just ran into it after passing around schema / tablenames as Strings and ending up with empty ResultSets for some DatabaseMetaData methods in some cases, so I checked the docs and did not find it mentioned there. dan> I didn't understand this paragraph, not sure what a "universal test" is, and this comment lost me "grant and have references privilege on a column for which it has no meaning to have that privilege" Sorry, it was late and I could have been more clear here. It is possible to grant references privilege on any column, not just primary keys or unique constraints. You can grant references on any column in the database, and Derby happily records that privilege, and you can see in the test that this privilege on the column can be confirmed from the metadata. I checked the SQL standard, grant for references seems to be allowed to be granted on arbitrary tables / columns in case those table / columns in the grant are at some point modified to have / be a primary key or have a unique constraint placed on them. However, having the privilege doesn't mean you can do anything useful with it without a primary key or unique constraint on the table / column, and there's no way to check the use of it without modifying the original table. i.e. in the style of the test: "create table foo (c1 int not null primary key, c2)" grant("references"..."c2"); assertReferencePrivilege("true"..."c2"); passes if you just check the metadata for the privilege, but you can't test in the assert method that you have references privilege on c2 without modifying table foo in the assert method. I feel this sort of violates the spirit of the assert tests, in that they shouldn't be changing the state of the things being asserted, just verifying their state. It's probably also why this check in the previous test was left stubbed out, since there's no good check for it that doesn't alter the original table's definition. It's not a problem, just making a note of it for the next person who comes along and wonders what is going on here. I'll go back and double-check the comments in the test to make sure they are clear.
          Hide
          Andrew McIntyre added a comment -

          Committed to trunk with revision 521432.

          Show
          Andrew McIntyre added a comment - Committed to trunk with revision 521432.
          Hide
          Andrew McIntyre added a comment -

          Closing this issue. I had hoped to run the grantRevokeDDL and grantRevokeDDL2 SQL scripts with an adapter, as the diffs between VMs are minimal, but they are not good candidates for running as ScriptTestCases because they make heavy use of multiple connections in ij, which causes problems when run with ij.runScript() since multiple connections don't work there. While it would be useful to convert these tests, they are very extensive and I will take a break from grant/revoke for the moment and convert a couple of other tests. I hope to come back to the remaining grant/revoke tests in the near future.

          Show
          Andrew McIntyre added a comment - Closing this issue. I had hoped to run the grantRevokeDDL and grantRevokeDDL2 SQL scripts with an adapter, as the diffs between VMs are minimal, but they are not good candidates for running as ScriptTestCases because they make heavy use of multiple connections in ij, which causes problems when run with ij.runScript() since multiple connections don't work there. While it would be useful to convert these tests, they are very extensive and I will take a break from grant/revoke for the moment and convert a couple of other tests. I hope to come back to the remaining grant/revoke tests in the near future.

            People

            • Assignee:
              Andrew McIntyre
              Reporter:
              Andrew McIntyre
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development