GitHub PullRequestReviewCommentEvent at 2014-06-09 14:31:06
Repository: conf-registration-api
Actor: hlbraddock
Payload: {"action":"created","comment":{"url":"https://api.github.com/repos/CruGlobal/conf-registration-api/pulls/comments/13542992","id":13542992,"diff_hunk":"@@ -80,9 +83,12 @@ public PostgresPGObjectToEnumConverter(Class<?> enumType)\n \t\t}\n \t}\n \n-\tpublic org.sql2o.Connection getTestSqlConnection()\n+\tpublic org.sql2o.Connection getTestSqlConnection(CrsProperties properties)\n \t{\n-\t\tConnection sqlConnection = new Connection(new Sql2o(\"jdbc:postgresql://localhost/crsdb\",\"crsuser\",\"crsuser\",QuirksMode.PostgreSQL));\n+\t\tConnection sqlConnection = new org.sql2o.Connection(new Sql2o(properties.getProperty(\"unittestDatabaseUrl\"),","path":"src/main/java/org/cru/crs/cdi/SqlConnectionProducer.java","position":20,"original_position":20,"commit_id":"61bdb0c66269b44c22107651331db6f2b59572c6","original_commit_id":"61bdb0c66269b44c22107651331db6f2b59572c6","user":{"login":"hlbraddock","id":2084351,"avatar_url":"https://avatars.githubusercontent.com/u/2084351?","gravatar_id":"851ec0d145bd8a00c484582e85f0f989","url":"https://api.github.com/users/hlbraddock","html_url":"https://github.com/hlbraddock","followers_url":"https://api.github.com/users/hlbraddock/followers","following_url":"https://api.github.com/users/hlbraddock/following{/other_user}","gists_url":"https://api.github.com/users/hlbraddock/gists{/gist_id}","starred_url":"https://api.github.com/users/hlbraddock/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hlbraddock/subscriptions","organizations_url":"https://api.github.com/users/hlbraddock/orgs","repos_url":"https://api.github.com/users/hlbraddock/repos","events_url":"https://api.github.com/users/hlbraddock/events{/privacy}","received_events_url":"https://api.github.com/users/hlbraddock/received_events","type":"User","site_admin":false},"body":"@ryancarlson What is the reason for not using already existing `database...` properties here instead of `unittestDatabase...`?","created_at":"2014-06-09T14:31:06Z","updated_at":"2014-06-09T14:31:06Z","html_url":"https://github.com/CruGlobal/conf-registration-api/pull/166#discussion_r13542992","pull_request_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/pulls/166","_links":{"self":{"href":"https://api.github.com/repos/CruGlobal/conf-registration-api/pulls/comments/13542992"},"html":{"href":"https://github.com/CruGlobal/conf-registration-api/pull/166#discussion_r13542992"},"pull_request":{"href":"https://api.github.com/repos/CruGlobal/conf-registration-api/pulls/166"}}},"pull_request":{"url":"https://api.github.com/repos/CruGlobal/conf-registration-api/pulls/166","id":16774836,"html_url":"https://github.com/CruGlobal/conf-registration-api/pull/166","diff_url":"https://github.com/CruGlobal/conf-registration-api/pull/166.diff","patch_url":"https://github.com/CruGlobal/conf-registration-api/pull/166.patch","issue_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/issues/166","number":166,"state":"open","title":"Database & Testing update","user":{"login":"ryancarlson","id":3768029,"avatar_url":"https://avatars.githubusercontent.com/u/3768029?","gravatar_id":"e08062f49f466891c09609353478cfbe","url":"https://api.github.com/users/ryancarlson","html_url":"https://github.com/ryancarlson","followers_url":"https://api.github.com/users/ryancarlson/followers","following_url":"https://api.github.com/users/ryancarlson/following{/other_user}","gists_url":"https://api.github.com/users/ryancarlson/gists{/gist_id}","starred_url":"https://api.github.com/users/ryancarlson/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ryancarlson/subscriptions","organizations_url":"https://api.github.com/users/ryancarlson/orgs","repos_url":"https://api.github.com/users/ryancarlson/repos","events_url":"https://api.github.com/users/ryancarlson/events{/privacy}","received_events_url":"https://api.github.com/users/ryancarlson/received_events","type":"User","site_admin":false},"body":"### In this PR are some structural changes to tests:\r\n - No database URLs or credentials are stored in the project (source code, config or otherwise)\r\n - All Database info & creds are retrieved via `CrsProperties`\r\n - All tests which interact with the database extend `AbstractTestWithDatabaseConnectivity`\r\n - `AbstractTestWithDatabaseConnectivity` does a refresh of the target database before each test run\r\n\r\n### Necessary changes:\r\n - Add properties which contain DB URL, user & password to filesystem properties file (usually found at /apps/apps-config/conf-registration-api-properties.xml).  See below for example.\r\n\r\n### Possible gotchas:\r\n - Do not EVER put production creds as the unittest properties like this, as a test run could wipe out the production database.  There is no reason to do so.  Production creds should reside only in the application properties.  Actually you could try, but I'll likely make it that the prod DB would ignore connections things other than app servers.\r\n\r\nExample properties:\r\n```xml\r\n    <!-- unittest database server connection info -->\r\n    <entry key=\"unittestDatabaseUrl\">jdbc:postgresql://localhost/eventhub_unittest</entry>\r\n    <entry key=\"unittestDatabaseUsername\">[username]</entry>\r\n    <entry key=\"unittestDatabasePassword\">[password]</entry>\r\n```","created_at":"2014-06-05T16:01:36Z","updated_at":"2014-06-09T14:31:06Z","closed_at":null,"merged_at":null,"merge_commit_sha":"6ef44157552b0dd4c928b2716d6ed28613731d72","assignee":{"login":"hlbraddock","id":2084351,"avatar_url":"https://avatars.githubusercontent.com/u/2084351?","gravatar_id":"851ec0d145bd8a00c484582e85f0f989","url":"https://api.github.com/users/hlbraddock","html_url":"https://github.com/hlbraddock","followers_url":"https://api.github.com/users/hlbraddock/followers","following_url":"https://api.github.com/users/hlbraddock/following{/other_user}","gists_url":"https://api.github.com/users/hlbraddock/gists{/gist_id}","starred_url":"https://api.github.com/users/hlbraddock/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/hlbraddock/subscriptions","organizations_url":"https://api.github.com/users/hlbraddock/orgs","repos_url":"https://api.github.com/users/hlbraddock/repos","events_url":"https://api.github.com/users/hlbraddock/events{/privacy}","received_events_url":"https://api.github.com/users/hlbraddock/received_events","type":"User","site_admin":false},"milestone":null,"commits_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/pulls/166/commits","review_comments_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/pulls/166/comments","review_comment_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/pulls/comments/{number}","comments_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/issues/166/comments","statuses_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/statuses/61bdb0c66269b44c22107651331db6f2b59572c6","head":{"label":"CruGlobal:improved-testing","ref":"improved-testing","sha":"61bdb0c66269b44c22107651331db6f2b59572c6","user":{"login":"CruGlobal","id":1785912,"avatar_url":"https://avatars.githubusercontent.com/u/1785912?","gravatar_id":"fff0e4838db6e8e33d895977195fa2ce","url":"https://api.github.com/users/CruGlobal","html_url":"https://github.com/CruGlobal","followers_url":"https://api.github.com/users/CruGlobal/followers","following_url":"https://api.github.com/users/CruGlobal/following{/other_user}","gists_url":"https://api.github.com/users/CruGlobal/gists{/gist_id}","starred_url":"https://api.github.com/users/CruGlobal/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/CruGlobal/subscriptions","organizations_url":"https://api.github.com/users/CruGlobal/orgs","repos_url":"https://api.github.com/users/CruGlobal/repos","events_url":"https://api.github.com/users/CruGlobal/events{/privacy}","received_events_url":"https://api.github.com/users/CruGlobal/received_events","type":"Organization","site_admin":false},"repo":{"id":10947678,"name":"conf-registration-api","full_name":"CruGlobal/conf-registration-api","owner":{"login":"CruGlobal","id":1785912,"avatar_url":"https://avatars.githubusercontent.com/u/1785912?","gravatar_id":"fff0e4838db6e8e33d895977195fa2ce","url":"https://api.github.com/users/CruGlobal","html_url":"https://github.com/CruGlobal","followers_url":"https://api.github.com/users/CruGlobal/followers","following_url":"https://api.github.com/users/CruGlobal/following{/other_user}","gists_url":"https://api.github.com/users/CruGlobal/gists{/gist_id}","starred_url":"https://api.github.com/users/CruGlobal/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/CruGlobal/subscriptions","organizations_url":"https://api.github.com/users/CruGlobal/orgs","repos_url":"https://api.github.com/users/CruGlobal/repos","events_url":"https://api.github.com/users/CruGlobal/events{/privacy}","received_events_url":"https://api.github.com/users/CruGlobal/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/CruGlobal/conf-registration-api","description":"eventhub api","fork":false,"url":"https://api.github.com/repos/CruGlobal/conf-registration-api","forks_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/forks","keys_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/keys{/key_id}","collaborators_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/teams","hooks_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/hooks","issue_events_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/issues/events{/number}","events_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/events","assignees_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/assignees{/user}","branches_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/branches{/branch}","tags_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/tags","blobs_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/git/refs{/sha}","trees_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/git/trees{/sha}","statuses_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/statuses/{sha}","languages_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/languages","stargazers_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/stargazers","contributors_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/contributors","subscribers_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/subscribers","subscription_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/subscription","commits_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/commits{/sha}","git_commits_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/git/commits{/sha}","comments_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/comments{/number}","issue_comment_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/issues/comments/{number}","contents_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/contents/{+path}","compare_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/compare/{base}...{head}","merges_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/merges","archive_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/downloads","issues_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/issues{/number}","pulls_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/pulls{/number}","milestones_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/milestones{/number}","notifications_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/labels{/name}","releases_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/releases{/id}","created_at":"2013-06-25T19:21:35Z","updated_at":"2014-06-06T21:32:28Z","pushed_at":"2014-06-06T21:32:28Z","git_url":"git://github.com/CruGlobal/conf-registration-api.git","ssh_url":"git@github.com:CruGlobal/conf-registration-api.git","clone_url":"https://github.com/CruGlobal/conf-registration-api.git","svn_url":"https://github.com/CruGlobal/conf-registration-api","homepage":"","size":11270,"stargazers_count":0,"watchers_count":0,"language":"Java","has_issues":true,"has_downloads":true,"has_wiki":true,"forks_count":3,"mirror_url":null,"open_issues_count":3,"forks":3,"open_issues":3,"watchers":0,"default_branch":"master"}},"base":{"label":"CruGlobal:master","ref":"master","sha":"954b29bbcfe6d919de734ddaf57d44fda626399b","user":{"login":"CruGlobal","id":1785912,"avatar_url":"https://avatars.githubusercontent.com/u/1785912?","gravatar_id":"fff0e4838db6e8e33d895977195fa2ce","url":"https://api.github.com/users/CruGlobal","html_url":"https://github.com/CruGlobal","followers_url":"https://api.github.com/users/CruGlobal/followers","following_url":"https://api.github.com/users/CruGlobal/following{/other_user}","gists_url":"https://api.github.com/users/CruGlobal/gists{/gist_id}","starred_url":"https://api.github.com/users/CruGlobal/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/CruGlobal/subscriptions","organizations_url":"https://api.github.com/users/CruGlobal/orgs","repos_url":"https://api.github.com/users/CruGlobal/repos","events_url":"https://api.github.com/users/CruGlobal/events{/privacy}","received_events_url":"https://api.github.com/users/CruGlobal/received_events","type":"Organization","site_admin":false},"repo":{"id":10947678,"name":"conf-registration-api","full_name":"CruGlobal/conf-registration-api","owner":{"login":"CruGlobal","id":1785912,"avatar_url":"https://avatars.githubusercontent.com/u/1785912?","gravatar_id":"fff0e4838db6e8e33d895977195fa2ce","url":"https://api.github.com/users/CruGlobal","html_url":"https://github.com/CruGlobal","followers_url":"https://api.github.com/users/CruGlobal/followers","following_url":"https://api.github.com/users/CruGlobal/following{/other_user}","gists_url":"https://api.github.com/users/CruGlobal/gists{/gist_id}","starred_url":"https://api.github.com/users/CruGlobal/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/CruGlobal/subscriptions","organizations_url":"https://api.github.com/users/CruGlobal/orgs","repos_url":"https://api.github.com/users/CruGlobal/repos","events_url":"https://api.github.com/users/CruGlobal/events{/privacy}","received_events_url":"https://api.github.com/users/CruGlobal/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/CruGlobal/conf-registration-api","description":"eventhub api","fork":false,"url":"https://api.github.com/repos/CruGlobal/conf-registration-api","forks_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/forks","keys_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/keys{/key_id}","collaborators_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/teams","hooks_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/hooks","issue_events_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/issues/events{/number}","events_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/events","assignees_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/assignees{/user}","branches_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/branches{/branch}","tags_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/tags","blobs_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/git/refs{/sha}","trees_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/git/trees{/sha}","statuses_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/statuses/{sha}","languages_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/languages","stargazers_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/stargazers","contributors_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/contributors","subscribers_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/subscribers","subscription_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/subscription","commits_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/commits{/sha}","git_commits_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/git/commits{/sha}","comments_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/comments{/number}","issue_comment_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/issues/comments/{number}","contents_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/contents/{+path}","compare_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/compare/{base}...{head}","merges_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/merges","archive_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/downloads","issues_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/issues{/number}","pulls_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/pulls{/number}","milestones_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/milestones{/number}","notifications_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/labels{/name}","releases_url":"https://api.github.com/repos/CruGlobal/conf-registration-api/releases{/id}","created_at":"2013-06-25T19:21:35Z","updated_at":"2014-06-06T21:32:28Z","pushed_at":"2014-06-06T21:32:28Z","git_url":"git://github.com/CruGlobal/conf-registration-api.git","ssh_url":"git@github.com:CruGlobal/conf-registration-api.git","clone_url":"https://github.com/CruGlobal/conf-registration-api.git","svn_url":"https://github.com/CruGlobal/conf-registration-api","homepage":"","size":11270,"stargazers_count":0,"watchers_count":0,"language":"Java","has_issues":true,"has_downloads":true,"has_wiki":true,"forks_count":3,"mirror_url":null,"open_issues_count":3,"forks":3,"open_issues":3,"watchers":0,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/CruGlobal/conf-registration-api/pulls/166"},"html":{"href":"https://github.com/CruGlobal/conf-registration-api/pull/166"},"issue":{"href":"https://api.github.com/repos/CruGlobal/conf-registration-api/issues/166"},"comments":{"href":"https://api.github.com/repos/CruGlobal/conf-registration-api/issues/166/comments"},"review_comments":{"href":"https://api.github.com/repos/CruGlobal/conf-registration-api/pulls/166/comments"},"review_comment":{"href":"https://api.github.com/repos/CruGlobal/conf-registration-api/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/CruGlobal/conf-registration-api/pulls/166/commits"},"statuses":{"href":"https://api.github.com/repos/CruGlobal/conf-registration-api/statuses/61bdb0c66269b44c22107651331db6f2b59572c6"}}}}