{"action":"created","issue":{"url":"https://api.github.com/repos/git-lfs/git-lfs/issues/1688","repository_url":"https://api.github.com/repos/git-lfs/git-lfs","labels_url":"https://api.github.com/repos/git-lfs/git-lfs/issues/1688/labels{/name}","comments_url":"https://api.github.com/repos/git-lfs/git-lfs/issues/1688/comments","events_url":"https://api.github.com/repos/git-lfs/git-lfs/issues/1688/events","html_url":"https://github.com/git-lfs/git-lfs/pull/1688","id":190420719,"number":1688,"title":"Catfilebatch unittests","user":{"login":"technoweenie","id":21,"avatar_url":"https://avatars.githubusercontent.com/u/21?v=3","gravatar_id":"","url":"https://api.github.com/users/technoweenie","html_url":"https://github.com/technoweenie","followers_url":"https://api.github.com/users/technoweenie/followers","following_url":"https://api.github.com/users/technoweenie/following{/other_user}","gists_url":"https://api.github.com/users/technoweenie/gists{/gist_id}","starred_url":"https://api.github.com/users/technoweenie/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/technoweenie/subscriptions","organizations_url":"https://api.github.com/users/technoweenie/orgs","repos_url":"https://api.github.com/users/technoweenie/repos","events_url":"https://api.github.com/users/technoweenie/events{/privacy}","received_events_url":"https://api.github.com/users/technoweenie/received_events","type":"User","site_admin":true},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2016-11-18T21:01:01Z","updated_at":"2016-11-18T23:02:56Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/git-lfs/git-lfs/pulls/1688","html_url":"https://github.com/git-lfs/git-lfs/pull/1688","diff_url":"https://github.com/git-lfs/git-lfs/pull/1688.diff","patch_url":"https://github.com/git-lfs/git-lfs/pull/1688.patch"},"body":"Added unit tests that would have caught the bug that required https://github.com/git-lfs/git-lfs/pull/1680.\r\n\r\nHere's the test run on a slightly old version of master, with these 2 commits cherry picked:\r\n\r\n```bash\r\n$ gl\r\ncommit 35509bfc8836bdd2f5493404caea28c257db90d4\r\nAuthor: risk danger olson <technoweenie@gmail.com>\r\nDate:   Fri Nov 18 13:48:41 2016 -0700\r\n\r\n    add unit tests for 'git cat-file --batch'\r\n\r\ncommit 65fdf9b6df5fabd19099528f18d95ced3301d333\r\nAuthor: risk danger olson <technoweenie@gmail.com>\r\nDate:   Fri Nov 18 12:54:56 2016 -0700\r\n\r\n    add unit tests for 'cat-file --batch-check'\r\n\r\ncommit f92bd3b2145b8504753d7b08c27b37befb7556eb\r\nAuthor: risk danger olson <technoweenie@gmail.com>\r\nDate:   Fri Nov 18 07:44:20 2016 -0700\r\n\r\n    remove deprecated features\r\n\r\n$ script/test lfs -run TestCatFileBatch\r\n--- FAIL: TestCatFileBatchScanner (0.00s)\r\n\tgitscanner_catfilebatchscanner_test.go:88: 0000000000000000000000000000000000000000 blob 1000\r\n\tgitscanner_catfilebatchscanner_test.go:88: 0000000000000000000000000000000000000000 blob 1000\r\n\tgitscanner_catfilebatchscanner_test.go:88: 0000000000000000000000000000000000000000 blob 1000\r\n\tgitscanner_catfilebatchscanner_test.go:88: 0000000000000000000000000000000000000000 blob 1000\r\n\tgitscanner_catfilebatchscanner_test.go:88: 0000000000000000000000000000000000000000 blob 1000\r\n\tgitscanner_catfilebatchscanner_test.go:88: 0000000000000000000000000000000000000000 blob 128\r\n\tgitscanner_catfilebatchscanner_test.go:88: 0000000000000000000000000000000000000000 blob 1000\r\n\tgitscanner_catfilebatchscanner_test.go:88: 0000000000000000000000000000000000000000 blob 1000\r\n\tgitscanner_catfilebatchscanner_test.go:88: 0000000000000000000000000000000000000000 blob 1000\r\n\tgitscanner_catfilebatchscanner_test.go:88: 0000000000000000000000000000000000000000 blob 1000\r\n\tgitscanner_catfilebatchscanner_test.go:88: 0000000000000000000000000000000000000000 blob 1000\r\n\tgitscanner_catfilebatchscanner_test.go:88: 0000000000000000000000000000000000000000 blob 128\r\n\tgitscanner_catfilebatchscanner_test.go:88: 0000000000000000000000000000000000000000 blob 1000\r\n\tgitscanner_catfilebatchscanner_test.go:88: 0000000000000000000000000000000000000000 blob 1000\r\n\tgitscanner_catfilebatchscanner_test.go:88: 0000000000000000000000000000000000000000 blob 1000\r\n\tgitscanner_catfilebatchscanner_test.go:88: 0000000000000000000000000000000000000000 blob 1000\r\n\tgitscanner_catfilebatchscanner_test.go:88: 0000000000000000000000000000000000000000 blob 1000\r\n        Error Trace:    gitscanner_catfilebatchscanner_test.go:50\r\n\tError:\t\tExpected nil, but got: git cat-file --batch:: Invalid: \"\\x01\\x84\\xe2\\xa6%\\x0e\\xaaN\\x04\\xc1.{K<[`\\xb8\\xc1\\x8fkf\\x1ds\\xcd15*\\xa5\\xf3\\x9fHWqG\\xe8\\xb7\\xe3\\x82F\\x1e\\x82\\xa1\\xf4\\xfcA\\x06m\\x8b=s\\x17K\\x9a2\\xbd7\\xc1\\xa7]\\xec\\x00;!\\xc2Y\\x9a?\\xc8\\xd0EE\\x1a\\xef|z\\xe4\\x04\\xbf$\\x87\\xa6\\x17Ӵ\\x97\\xd2\\r\\xbb\\xb7^\\xe7\\xa5\\xcc9F,\\xeb߅&\\n\"\r\n\r\nFAIL\r\nFAIL\tgithub.com/git-lfs/git-lfs/lfs\t0.010s\r\n```\r\n\r\nIt's reading the invalid data because `io.LimitReader` wasn't ensuring that the `catFileBatchScanner` is reading all of the non-lfs pointer bytes."},"comment":{"url":"https://api.github.com/repos/git-lfs/git-lfs/issues/comments/261664960","html_url":"https://github.com/git-lfs/git-lfs/pull/1688#issuecomment-261664960","issue_url":"https://api.github.com/repos/git-lfs/git-lfs/issues/1688","id":261664960,"user":{"login":"technoweenie","id":21,"avatar_url":"https://avatars.githubusercontent.com/u/21?v=3","gravatar_id":"","url":"https://api.github.com/users/technoweenie","html_url":"https://github.com/technoweenie","followers_url":"https://api.github.com/users/technoweenie/followers","following_url":"https://api.github.com/users/technoweenie/following{/other_user}","gists_url":"https://api.github.com/users/technoweenie/gists{/gist_id}","starred_url":"https://api.github.com/users/technoweenie/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/technoweenie/subscriptions","organizations_url":"https://api.github.com/users/technoweenie/orgs","repos_url":"https://api.github.com/users/technoweenie/repos","events_url":"https://api.github.com/users/technoweenie/events{/privacy}","received_events_url":"https://api.github.com/users/technoweenie/received_events","type":"User","site_admin":true},"created_at":"2016-11-18T23:02:56Z","updated_at":"2016-11-18T23:02:56Z","body":"All 3 of these have a similar pattern for running commands. They spin up a `*exec.Cmd`, start up a goroutine that synchronously passes things to stdout, waiting for it to return something via stdout.\r\n\r\n* `runCatFileBatchCheck`\r\n* `runCatFileBatch`\r\n* `catFileBatchTree`\r\n\r\nI want to see if I can clean up `lsTreeBlobs` a bit and get rid of the duplicate `catFileBatchTree` altogether."}}
{"actor":{"display_login":"technoweenie"}}