)]}'
{"java/src/main/java/monasca/api/infrastructure/persistence/mysql/NotificationMethodMySqlRepoImpl.java":[{"author":{"_account_id":16168,"name":"Tomasz Trębski","email":"kornicameister@gmail.com","username":"trebskit"},"change_message_id":"002256da324ae68c1b68632cd2e120aae793429f","unresolved":false,"context_lines":[{"line_number":224,"context_line":"            notificationMethodId);"},{"line_number":225,"context_line":"      h.commit();"},{"line_number":226,"context_line":"      return new NotificationMethod(notificationMethodId, name, notificationMethodType, address, period);"},{"line_number":227,"context_line":"    } catch (RuntimeException e) {"},{"line_number":228,"context_line":"      h.rollback();"},{"line_number":229,"context_line":"      throw e;"},{"line_number":230,"context_line":"    } finally {"}],"source_content_type":"text/x-java","patch_set":1,"id":"fa7ab95a_9af9accb","line":227,"updated":"2016-08-26 08:24:37.000000000","message":"Can we add unit-tests for this case ?","commit_id":"2726afb9854ae5eeb2d7c73e2c961231d3db7111"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"07597b1fea3068c8f4496fe1b8c51ff0933ba80d","unresolved":false,"context_lines":[{"line_number":224,"context_line":"            notificationMethodId);"},{"line_number":225,"context_line":"      h.commit();"},{"line_number":226,"context_line":"      return new NotificationMethod(notificationMethodId, name, notificationMethodType, address, period);"},{"line_number":227,"context_line":"    } catch (RuntimeException e) {"},{"line_number":228,"context_line":"      h.rollback();"},{"line_number":229,"context_line":"      throw e;"},{"line_number":230,"context_line":"    } finally {"}],"source_content_type":"text/x-java","patch_set":1,"id":"fa7ab95a_a75b5e2d","line":227,"in_reply_to":"fa7ab95a_4f9d7558","updated":"2016-08-29 03:58:12.000000000","message":"I used Mockito.spy() to verify the transaction is closed. That was interesting to figure out how to do it.","commit_id":"2726afb9854ae5eeb2d7c73e2c961231d3db7111"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"fa8274c20107e3b157246729e7af915a14774357","unresolved":false,"context_lines":[{"line_number":224,"context_line":"            notificationMethodId);"},{"line_number":225,"context_line":"      h.commit();"},{"line_number":226,"context_line":"      return new NotificationMethod(notificationMethodId, name, notificationMethodType, address, period);"},{"line_number":227,"context_line":"    } catch (RuntimeException e) {"},{"line_number":228,"context_line":"      h.rollback();"},{"line_number":229,"context_line":"      throw e;"},{"line_number":230,"context_line":"    } finally {"}],"source_content_type":"text/x-java","patch_set":1,"id":"fa7ab95a_c75fb770","line":227,"in_reply_to":"fa7ab95a_9af9accb","updated":"2016-08-26 16:19:42.000000000","message":"I tried, but the driver used for the unit tests must not have the same behavior as the real mysql driver. I tried using assertFalse(db.open().isInTransaction()) with the old code, but it did not fail. I hesitate to add a test that doesn\u0027t actually validate the bug.","commit_id":"2726afb9854ae5eeb2d7c73e2c961231d3db7111"},{"author":{"_account_id":16168,"name":"Tomasz Trębski","email":"kornicameister@gmail.com","username":"trebskit"},"change_message_id":"fc16e62eae7c87c78e0bcc2d94e5b1fbd2c2b790","unresolved":false,"context_lines":[{"line_number":224,"context_line":"            notificationMethodId);"},{"line_number":225,"context_line":"      h.commit();"},{"line_number":226,"context_line":"      return new NotificationMethod(notificationMethodId, name, notificationMethodType, address, period);"},{"line_number":227,"context_line":"    } catch (RuntimeException e) {"},{"line_number":228,"context_line":"      h.rollback();"},{"line_number":229,"context_line":"      throw e;"},{"line_number":230,"context_line":"    } finally {"}],"source_content_type":"text/x-java","patch_set":1,"id":"fa7ab95a_4f9d7558","line":227,"in_reply_to":"fa7ab95a_c75fb770","updated":"2016-08-26 20:29:09.000000000","message":"Well, I thought that this is actually about checking if transaction was rollbacked. So I would expect a unit test that just sees if rollback/close was called if anything triggered RuntimeException inside try.catch block.","commit_id":"2726afb9854ae5eeb2d7c73e2c961231d3db7111"}],"java/src/test/java/monasca/api/infrastructure/persistence/mysql/NotificationMethodMySqlRepositoryImplTest.java":[{"author":{"_account_id":16168,"name":"Tomasz Trębski","email":"kornicameister@gmail.com","username":"trebskit"},"change_message_id":"cab6d4b4676ffb48768fcd8440e94fe5bf074ed1","unresolved":false,"context_lines":[{"line_number":112,"context_line":"  @AfterMethod"},{"line_number":113,"context_line":"  protected void afterMethod() {"},{"line_number":114,"context_line":"    if (shouldRollback) {"},{"line_number":115,"context_line":"      verify(spyHandle, times(1)).rollback();"},{"line_number":116,"context_line":"    }"},{"line_number":117,"context_line":"  }"},{"line_number":118,"context_line":""}],"source_content_type":"text/x-java","patch_set":3,"id":"fa7ab95a_93ba1c6d","line":115,"updated":"2016-08-29 06:48:43.000000000","message":"Otherwise that would be\n\n verify(spyHandler, times(0)).rollback();\n\nright ?\n\nMaybe a better idea would be to have code like:\n\n verify(spyHandler, times(shouldRollback ? 1 : 0)).rollback()\n\nto make sure code did not reach rollback block.","commit_id":"be0b5874b131bbf3eeaae9ac43d0a5ade3103fba"},{"author":{"_account_id":16168,"name":"Tomasz Trębski","email":"kornicameister@gmail.com","username":"trebskit"},"change_message_id":"85134978dc2b32864fa319d26219cfee09a5ac61","unresolved":false,"context_lines":[{"line_number":112,"context_line":"  @AfterMethod"},{"line_number":113,"context_line":"  protected void afterMethod() {"},{"line_number":114,"context_line":"    if (shouldRollback) {"},{"line_number":115,"context_line":"      verify(spyHandle, times(1)).rollback();"},{"line_number":116,"context_line":"    }"},{"line_number":117,"context_line":"  }"},{"line_number":118,"context_line":""}],"source_content_type":"text/x-java","patch_set":3,"id":"fa7ab95a_a4bddf69","line":115,"in_reply_to":"fa7ab95a_15a97024","updated":"2016-08-29 21:18:25.000000000","message":"Oh yeah...I already got confused by that answer ;).","commit_id":"be0b5874b131bbf3eeaae9ac43d0a5ade3103fba"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"1a59320a2709cb2197304e06847661750e359733","unresolved":false,"context_lines":[{"line_number":112,"context_line":"  @AfterMethod"},{"line_number":113,"context_line":"  protected void afterMethod() {"},{"line_number":114,"context_line":"    if (shouldRollback) {"},{"line_number":115,"context_line":"      verify(spyHandle, times(1)).rollback();"},{"line_number":116,"context_line":"    }"},{"line_number":117,"context_line":"  }"},{"line_number":118,"context_line":""}],"source_content_type":"text/x-java","patch_set":3,"id":"fa7ab95a_15a97024","line":115,"in_reply_to":"fa7ab95a_93ba1c6d","updated":"2016-08-29 15:41:49.000000000","message":"It is a little more complex than that because the tests that can do a rollback actually open two handles: one to do the create or update, and another one to read from the database to ensure the object was stored correctly. So, in the successful case, spyhandle is actually the second handle, not the one that had the commit or rollback.\n\nI could change the code to make that test in the doAnswer above, but that is already hard to follow and adding more to it would make it harder yet to understand. I think we are getting the past the point of diminishing returns.","commit_id":"be0b5874b131bbf3eeaae9ac43d0a5ade3103fba"}]}