{"action":"created","comment":{"url":"https://api.github.com/repos/confluentinc/kafka-rest/pulls/comments/47521685","id":47521685,"diff_hunk":"@@ -172,38 +177,52 @@ public void testConsumerNormalOps() throws InterruptedException, ExecutionExcept\n     String cid = consumerManager.createConsumer(\n         groupName, new ConsumerInstanceConfig(EmbeddedFormat.BINARY));\n     sawCallback = false;\n+    actualException = null;\n+    actualRecords = null;\n     consumerManager.readTopic(\n         groupName, cid, topicName, BinaryConsumerState.class, Long.MAX_VALUE,\n         new ConsumerManager.ReadCallback<byte[], byte[]>() {\n           @Override\n           public void onCompletion(List<? extends ConsumerRecord<byte[], byte[]>> records,\n                                    Exception e) {\n+            actualException = e;\n+            actualRecords = records;\n             sawCallback = true;\n-            assertNull(e);\n-            assertEquals(referenceRecords, records);\n+\n+            // Assert that should clearly fail...but doesn't report in test runs\n+            assertTrue(\"This should fail\", false);\n           }\n         }).get();\n-    assertTrue(sawCallback);\n+    assertTrue(\"Callback failed to fire\", sawCallback);\n+    assertNull(\"No exception in callback\", actualException);\n+    assertEquals(\"Records returned not as expected\", referenceRecords, actualRecords);\n+\n     // With # of bytes in messages < max bytes per response, this should finish just after\n-    // the per-request timeout (because the timeout perfectly coincides with a scheduled\n-    // iteration when using the default settings).\n-    assertEquals(config.getInt(KafkaRestConfig.CONSUMER_REQUEST_TIMEOUT_MS_CONFIG)\n-                 + config.getInt(KafkaRestConfig.CONSUMER_ITERATOR_TIMEOUT_MS_CONFIG),\n-                 config.getTime().milliseconds());\n+    // the per-request timeout (attempts to tie this down to the ms not possible so assumes that greater than is \"good enough\")\n+    String msg = \"Time taken (\" + Long.toString(config.getTime().milliseconds()) + \") to process message should be greater than the timeout \" +\n+        Integer.toString(config.getInt(KafkaRestConfig.CONSUMER_REQUEST_TIMEOUT_MS_CONFIG)\n+                 + config.getInt(KafkaRestConfig.CONSUMER_ITERATOR_TIMEOUT_MS_CONFIG)) ;\n+    assertFalse(msg, config.getInt(KafkaRestConfig.CONSUMER_REQUEST_TIMEOUT_MS_CONFIG)","path":"src/test/java/io/confluent/kafkarest/unit/ConsumerManagerTest.java","position":50,"original_position":50,"commit_id":"000fa18ed78f00b96206404f085838bc070142c4","original_commit_id":"000fa18ed78f00b96206404f085838bc070142c4","user":{"login":"gawth","id":5629725,"avatar_url":"https://avatars.githubusercontent.com/u/5629725?v=3","gravatar_id":"","url":"https://api.github.com/users/gawth","html_url":"https://github.com/gawth","followers_url":"https://api.github.com/users/gawth/followers","following_url":"https://api.github.com/users/gawth/following{/other_user}","gists_url":"https://api.github.com/users/gawth/gists{/gist_id}","starred_url":"https://api.github.com/users/gawth/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gawth/subscriptions","organizations_url":"https://api.github.com/users/gawth/orgs","repos_url":"https://api.github.com/users/gawth/repos","events_url":"https://api.github.com/users/gawth/events{/privacy}","received_events_url":"https://api.github.com/users/gawth/received_events","type":"User","site_admin":false},"body":"Because the MockTime now relies on an increment inside the milliseconds method its no longer possible to be exact on time comparison...","created_at":"2015-12-14T16:45:37Z","updated_at":"2015-12-14T16:45:37Z","html_url":"https://github.com/confluentinc/kafka-rest/pull/127#discussion_r47521685","pull_request_url":"https://api.github.com/repos/confluentinc/kafka-rest/pulls/127","_links":{"self":{"href":"https://api.github.com/repos/confluentinc/kafka-rest/pulls/comments/47521685"},"html":{"href":"https://github.com/confluentinc/kafka-rest/pull/127#discussion_r47521685"},"pull_request":{"href":"https://api.github.com/repos/confluentinc/kafka-rest/pulls/127"}}},"pull_request":{"url":"https://api.github.com/repos/confluentinc/kafka-rest/pulls/127","id":51678065,"html_url":"https://github.com/confluentinc/kafka-rest/pull/127","diff_url":"https://github.com/confluentinc/kafka-rest/pull/127.diff","patch_url":"https://github.com/confluentinc/kafka-rest/pull/127.patch","issue_url":"https://api.github.com/repos/confluentinc/kafka-rest/issues/127","number":127,"state":"open","locked":false,"title":"Fix to ConsumerManager tests","user":{"login":"gawth","id":5629725,"avatar_url":"https://avatars.githubusercontent.com/u/5629725?v=3","gravatar_id":"","url":"https://api.github.com/users/gawth","html_url":"https://github.com/gawth","followers_url":"https://api.github.com/users/gawth/followers","following_url":"https://api.github.com/users/gawth/following{/other_user}","gists_url":"https://api.github.com/users/gawth/gists{/gist_id}","starred_url":"https://api.github.com/users/gawth/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gawth/subscriptions","organizations_url":"https://api.github.com/users/gawth/orgs","repos_url":"https://api.github.com/users/gawth/repos","events_url":"https://api.github.com/users/gawth/events{/privacy}","received_events_url":"https://api.github.com/users/gawth/received_events","type":"User","site_admin":false},"body":"Hi,\r\n\r\nwhilst looking into a bug related to long polling I discovered that the consumer manager tests have a flaw in them.\r\n\r\nThe tests rely on assertions within a callback method.  However, assertion fails are hidden and the test code continues without reporting a test failure.\r\n\r\nI have fixed the tests but left an assert that should clearly fail as a demo of the issue (see testConsumerNormalOps, first test case).\r\n\r\nI'm not a Java dev by trade so you may want to provide further guidance on how you'd prefer to restructure the assertions.  My basic approach is to save off the results of the callback to variables that are accessible outside the callback and then carry out assertion tests (as per existing sawCallback checks).\r\n\r\nHaving fixed the flaw in the tests one of the tests (testConsumerExceptions) now fails.  Not sure if this is a code or a test problem - happy to fix it but need some guidance on what is required.\r\n \r\nThanks,\r\nAlan","created_at":"2015-11-24T15:57:47Z","updated_at":"2015-12-14T16:45:38Z","closed_at":null,"merged_at":null,"merge_commit_sha":"db7f5d504d4a4f5f00c433046cb712f1a47dee33","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/confluentinc/kafka-rest/pulls/127/commits","review_comments_url":"https://api.github.com/repos/confluentinc/kafka-rest/pulls/127/comments","review_comment_url":"https://api.github.com/repos/confluentinc/kafka-rest/pulls/comments{/number}","comments_url":"https://api.github.com/repos/confluentinc/kafka-rest/issues/127/comments","statuses_url":"https://api.github.com/repos/confluentinc/kafka-rest/statuses/000fa18ed78f00b96206404f085838bc070142c4","head":{"label":"gawth:master","ref":"master","sha":"000fa18ed78f00b96206404f085838bc070142c4","user":{"login":"gawth","id":5629725,"avatar_url":"https://avatars.githubusercontent.com/u/5629725?v=3","gravatar_id":"","url":"https://api.github.com/users/gawth","html_url":"https://github.com/gawth","followers_url":"https://api.github.com/users/gawth/followers","following_url":"https://api.github.com/users/gawth/following{/other_user}","gists_url":"https://api.github.com/users/gawth/gists{/gist_id}","starred_url":"https://api.github.com/users/gawth/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gawth/subscriptions","organizations_url":"https://api.github.com/users/gawth/orgs","repos_url":"https://api.github.com/users/gawth/repos","events_url":"https://api.github.com/users/gawth/events{/privacy}","received_events_url":"https://api.github.com/users/gawth/received_events","type":"User","site_admin":false},"repo":{"id":46782736,"name":"kafka-rest","full_name":"gawth/kafka-rest","owner":{"login":"gawth","id":5629725,"avatar_url":"https://avatars.githubusercontent.com/u/5629725?v=3","gravatar_id":"","url":"https://api.github.com/users/gawth","html_url":"https://github.com/gawth","followers_url":"https://api.github.com/users/gawth/followers","following_url":"https://api.github.com/users/gawth/following{/other_user}","gists_url":"https://api.github.com/users/gawth/gists{/gist_id}","starred_url":"https://api.github.com/users/gawth/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gawth/subscriptions","organizations_url":"https://api.github.com/users/gawth/orgs","repos_url":"https://api.github.com/users/gawth/repos","events_url":"https://api.github.com/users/gawth/events{/privacy}","received_events_url":"https://api.github.com/users/gawth/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/gawth/kafka-rest","description":"REST Proxy for Kafka","fork":true,"url":"https://api.github.com/repos/gawth/kafka-rest","forks_url":"https://api.github.com/repos/gawth/kafka-rest/forks","keys_url":"https://api.github.com/repos/gawth/kafka-rest/keys{/key_id}","collaborators_url":"https://api.github.com/repos/gawth/kafka-rest/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/gawth/kafka-rest/teams","hooks_url":"https://api.github.com/repos/gawth/kafka-rest/hooks","issue_events_url":"https://api.github.com/repos/gawth/kafka-rest/issues/events{/number}","events_url":"https://api.github.com/repos/gawth/kafka-rest/events","assignees_url":"https://api.github.com/repos/gawth/kafka-rest/assignees{/user}","branches_url":"https://api.github.com/repos/gawth/kafka-rest/branches{/branch}","tags_url":"https://api.github.com/repos/gawth/kafka-rest/tags","blobs_url":"https://api.github.com/repos/gawth/kafka-rest/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/gawth/kafka-rest/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/gawth/kafka-rest/git/refs{/sha}","trees_url":"https://api.github.com/repos/gawth/kafka-rest/git/trees{/sha}","statuses_url":"https://api.github.com/repos/gawth/kafka-rest/statuses/{sha}","languages_url":"https://api.github.com/repos/gawth/kafka-rest/languages","stargazers_url":"https://api.github.com/repos/gawth/kafka-rest/stargazers","contributors_url":"https://api.github.com/repos/gawth/kafka-rest/contributors","subscribers_url":"https://api.github.com/repos/gawth/kafka-rest/subscribers","subscription_url":"https://api.github.com/repos/gawth/kafka-rest/subscription","commits_url":"https://api.github.com/repos/gawth/kafka-rest/commits{/sha}","git_commits_url":"https://api.github.com/repos/gawth/kafka-rest/git/commits{/sha}","comments_url":"https://api.github.com/repos/gawth/kafka-rest/comments{/number}","issue_comment_url":"https://api.github.com/repos/gawth/kafka-rest/issues/comments{/number}","contents_url":"https://api.github.com/repos/gawth/kafka-rest/contents/{+path}","compare_url":"https://api.github.com/repos/gawth/kafka-rest/compare/{base}...{head}","merges_url":"https://api.github.com/repos/gawth/kafka-rest/merges","archive_url":"https://api.github.com/repos/gawth/kafka-rest/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/gawth/kafka-rest/downloads","issues_url":"https://api.github.com/repos/gawth/kafka-rest/issues{/number}","pulls_url":"https://api.github.com/repos/gawth/kafka-rest/pulls{/number}","milestones_url":"https://api.github.com/repos/gawth/kafka-rest/milestones{/number}","notifications_url":"https://api.github.com/repos/gawth/kafka-rest/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/gawth/kafka-rest/labels{/name}","releases_url":"https://api.github.com/repos/gawth/kafka-rest/releases{/id}","created_at":"2015-11-24T09:58:46Z","updated_at":"2015-11-24T09:58:47Z","pushed_at":"2015-12-14T16:30:04Z","git_url":"git://github.com/gawth/kafka-rest.git","ssh_url":"git@github.com:gawth/kafka-rest.git","clone_url":"https://github.com/gawth/kafka-rest.git","svn_url":"https://github.com/gawth/kafka-rest","homepage":null,"size":806,"stargazers_count":0,"watchers_count":0,"language":"Java","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"confluentinc:master","ref":"master","sha":"b2f5b18ddfd7336e8462fdd2fd710561b37c849b","user":{"login":"confluentinc","id":9439498,"avatar_url":"https://avatars.githubusercontent.com/u/9439498?v=3","gravatar_id":"","url":"https://api.github.com/users/confluentinc","html_url":"https://github.com/confluentinc","followers_url":"https://api.github.com/users/confluentinc/followers","following_url":"https://api.github.com/users/confluentinc/following{/other_user}","gists_url":"https://api.github.com/users/confluentinc/gists{/gist_id}","starred_url":"https://api.github.com/users/confluentinc/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/confluentinc/subscriptions","organizations_url":"https://api.github.com/users/confluentinc/orgs","repos_url":"https://api.github.com/users/confluentinc/repos","events_url":"https://api.github.com/users/confluentinc/events{/privacy}","received_events_url":"https://api.github.com/users/confluentinc/received_events","type":"Organization","site_admin":false},"repo":{"id":26873147,"name":"kafka-rest","full_name":"confluentinc/kafka-rest","owner":{"login":"confluentinc","id":9439498,"avatar_url":"https://avatars.githubusercontent.com/u/9439498?v=3","gravatar_id":"","url":"https://api.github.com/users/confluentinc","html_url":"https://github.com/confluentinc","followers_url":"https://api.github.com/users/confluentinc/followers","following_url":"https://api.github.com/users/confluentinc/following{/other_user}","gists_url":"https://api.github.com/users/confluentinc/gists{/gist_id}","starred_url":"https://api.github.com/users/confluentinc/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/confluentinc/subscriptions","organizations_url":"https://api.github.com/users/confluentinc/orgs","repos_url":"https://api.github.com/users/confluentinc/repos","events_url":"https://api.github.com/users/confluentinc/events{/privacy}","received_events_url":"https://api.github.com/users/confluentinc/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/confluentinc/kafka-rest","description":"REST Proxy for Kafka","fork":false,"url":"https://api.github.com/repos/confluentinc/kafka-rest","forks_url":"https://api.github.com/repos/confluentinc/kafka-rest/forks","keys_url":"https://api.github.com/repos/confluentinc/kafka-rest/keys{/key_id}","collaborators_url":"https://api.github.com/repos/confluentinc/kafka-rest/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/confluentinc/kafka-rest/teams","hooks_url":"https://api.github.com/repos/confluentinc/kafka-rest/hooks","issue_events_url":"https://api.github.com/repos/confluentinc/kafka-rest/issues/events{/number}","events_url":"https://api.github.com/repos/confluentinc/kafka-rest/events","assignees_url":"https://api.github.com/repos/confluentinc/kafka-rest/assignees{/user}","branches_url":"https://api.github.com/repos/confluentinc/kafka-rest/branches{/branch}","tags_url":"https://api.github.com/repos/confluentinc/kafka-rest/tags","blobs_url":"https://api.github.com/repos/confluentinc/kafka-rest/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/confluentinc/kafka-rest/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/confluentinc/kafka-rest/git/refs{/sha}","trees_url":"https://api.github.com/repos/confluentinc/kafka-rest/git/trees{/sha}","statuses_url":"https://api.github.com/repos/confluentinc/kafka-rest/statuses/{sha}","languages_url":"https://api.github.com/repos/confluentinc/kafka-rest/languages","stargazers_url":"https://api.github.com/repos/confluentinc/kafka-rest/stargazers","contributors_url":"https://api.github.com/repos/confluentinc/kafka-rest/contributors","subscribers_url":"https://api.github.com/repos/confluentinc/kafka-rest/subscribers","subscription_url":"https://api.github.com/repos/confluentinc/kafka-rest/subscription","commits_url":"https://api.github.com/repos/confluentinc/kafka-rest/commits{/sha}","git_commits_url":"https://api.github.com/repos/confluentinc/kafka-rest/git/commits{/sha}","comments_url":"https://api.github.com/repos/confluentinc/kafka-rest/comments{/number}","issue_comment_url":"https://api.github.com/repos/confluentinc/kafka-rest/issues/comments{/number}","contents_url":"https://api.github.com/repos/confluentinc/kafka-rest/contents/{+path}","compare_url":"https://api.github.com/repos/confluentinc/kafka-rest/compare/{base}...{head}","merges_url":"https://api.github.com/repos/confluentinc/kafka-rest/merges","archive_url":"https://api.github.com/repos/confluentinc/kafka-rest/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/confluentinc/kafka-rest/downloads","issues_url":"https://api.github.com/repos/confluentinc/kafka-rest/issues{/number}","pulls_url":"https://api.github.com/repos/confluentinc/kafka-rest/pulls{/number}","milestones_url":"https://api.github.com/repos/confluentinc/kafka-rest/milestones{/number}","notifications_url":"https://api.github.com/repos/confluentinc/kafka-rest/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/confluentinc/kafka-rest/labels{/name}","releases_url":"https://api.github.com/repos/confluentinc/kafka-rest/releases{/id}","created_at":"2014-11-19T17:34:24Z","updated_at":"2015-12-11T09:45:33Z","pushed_at":"2015-12-14T16:30:04Z","git_url":"git://github.com/confluentinc/kafka-rest.git","ssh_url":"git@github.com:confluentinc/kafka-rest.git","clone_url":"https://github.com/confluentinc/kafka-rest.git","svn_url":"https://github.com/confluentinc/kafka-rest","homepage":null,"size":943,"stargazers_count":152,"watchers_count":152,"language":"Java","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":58,"mirror_url":null,"open_issues_count":41,"forks":58,"open_issues":41,"watchers":152,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/confluentinc/kafka-rest/pulls/127"},"html":{"href":"https://github.com/confluentinc/kafka-rest/pull/127"},"issue":{"href":"https://api.github.com/repos/confluentinc/kafka-rest/issues/127"},"comments":{"href":"https://api.github.com/repos/confluentinc/kafka-rest/issues/127/comments"},"review_comments":{"href":"https://api.github.com/repos/confluentinc/kafka-rest/pulls/127/comments"},"review_comment":{"href":"https://api.github.com/repos/confluentinc/kafka-rest/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/confluentinc/kafka-rest/pulls/127/commits"},"statuses":{"href":"https://api.github.com/repos/confluentinc/kafka-rest/statuses/000fa18ed78f00b96206404f085838bc070142c4"}}}}