Merge lp:~openerp-community-testers/openobject-server/trunk-sequences-test-bug-1083724 into lp:openobject-server

[Georges Racinet]

Checking the error code. This pattern of a posteriori checking of the exception arguments after using self.assertRaises as a context manager is actually inspired by python documentation (http://docs.python.org/dev/library/unittest.html#unittest.TestCase.assertRaises)

Also now cursor closing occurs in a finally: statement to avoid blocking besides failing

[Vo Minh Thu (thu)]

Hi!

Indeed, the

  with ... as raise_ctx:

seems nice.

But the diff seems messed up by tabs.

And I don't understand the try/finally. Don't you want to put all the code using the cursors in the try body?

review: Needs Information

[Vo Minh Thu (thu)]

(Please put back the merge prop to 'need review' if necessary.)

[Georges Racinet (gracinet)]

Ah yes, sorry about the tabs, I worked on it directly on a dumb server.

About the try/finally, I thought that afterwards, too.

Note that most tests from that module should have their cursor closing similarly protected, as well.

[Georges Racinet (gracinet)]

Should be better, now.

I confirm that cursor treatment could really be improved in these tests:
  - no protection against failures, and we've seen that can leave locks behind
  - commits !

I imagine things could be really improved by subclassing TransactionCase (which I guess is more recent) except for these draw_twice.

But that should be a separate issue.

Unmerged revisions

4604. By Georges Racinet <email address hidden> on 2012-11-29

A wider try/finally in test draw_twice (no gap)

4603. By Georges Racinet <email address hidden> on 2012-11-29

Getting rid of tabs, plus one pyflakes error

4602. By Georges Racinet <email address hidden> on 2012-11-27

[FIX] Proper checking of pg error code for expected lock fail in test

=== modified file 'openerp/tests/test_ir_sequence.py'
--- openerp/tests/test_ir_sequence.py	2012-11-28 13:51:01 +0000
+++ openerp/tests/test_ir_sequence.py	2012-11-29 13:45:30 +0000
@@ -110,14 +110,21 @@
         """
         cr0 = cursor()
         cr1 = cursor()
-        cr1._default_log_exceptions = False # Prevent logging a traceback
-        msg_re = '^could not obtain lock on row in relation "ir_sequence"$'
-        with self.assertRaisesRegexp(psycopg2.OperationalError, msg_re):
-            n0 = registry('ir.sequence').next_by_code(cr0, ADMIN_USER_ID, 'test_sequence_type_2', {})
-            assert n0
-            n1 = registry('ir.sequence').next_by_code(cr1, ADMIN_USER_ID, 'test_sequence_type_2', {})
-        cr0.close()
-        cr1.close()
+        try:
+            cr1._default_log_exceptions = False # Prevent logging a traceback
+            with self.assertRaises(psycopg2.OperationalError) as raise_ctx:
+                n0 = registry('ir.sequence').next_by_code(cr0, ADMIN_USER_ID, 'test_sequence_type_2', {})
+                assert n0
+                registry('ir.sequence').next_by_code(cr1, ADMIN_USER_ID, 'test_sequence_type_2', {})
+
+            exc = raise_ctx.exception
+            self.assertEquals(exc.pgcode,
+                              psycopg2.errorcodes.LOCK_NOT_AVAILABLE,
+                              "Unexpected PostgreSQL Error instead of "
+                              "expected LOCK_NOT_AVAILABLE :" + exc.pgerror)
+        finally:
+            cr0.close()
+            cr1.close()
 
     @classmethod
     def tearDownClass(cls):