{"action":"created","issue":{"url":"https://api.github.com/repos/apiaryio/dredd/issues/332","repository_url":"https://api.github.com/repos/apiaryio/dredd","labels_url":"https://api.github.com/repos/apiaryio/dredd/issues/332/labels{/name}","comments_url":"https://api.github.com/repos/apiaryio/dredd/issues/332/comments","events_url":"https://api.github.com/repos/apiaryio/dredd/issues/332/events","html_url":"https://github.com/apiaryio/dredd/pull/332","id":124035914,"number":332,"title":"Big refactoring and optimisation of hook handler spawner and client","user":{"login":"netmilk","id":79609,"avatar_url":"https://avatars.githubusercontent.com/u/79609?v=3","gravatar_id":"","url":"https://api.github.com/users/netmilk","html_url":"https://github.com/netmilk","followers_url":"https://api.github.com/users/netmilk/followers","following_url":"https://api.github.com/users/netmilk/following{/other_user}","gists_url":"https://api.github.com/users/netmilk/gists{/gist_id}","starred_url":"https://api.github.com/users/netmilk/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/netmilk/subscriptions","organizations_url":"https://api.github.com/users/netmilk/orgs","repos_url":"https://api.github.com/users/netmilk/repos","events_url":"https://api.github.com/users/netmilk/events{/privacy}","received_events_url":"https://api.github.com/users/netmilk/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/apiaryio/dredd/labels/in%20progress","name":"in progress","color":"e11d21"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":10,"created_at":"2015-12-28T08:44:39Z","updated_at":"2016-02-23T14:18:43Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/apiaryio/dredd/pulls/332","html_url":"https://github.com/apiaryio/dredd/pull/332","diff_url":"https://github.com/apiaryio/dredd/pull/332.diff","patch_url":"https://github.com/apiaryio/dredd/pull/332.patch"},"body":"# * * *  WIP DO NOT MERGE * * * \r\n\r\n- [x] Removed proxyquire hell, using clean, asynchronous code (#235)\r\n- [x] Gracefully handling handler crashes and kills and lot of corner-cases (#238)\r\n- [x] Removed console.logging using logger or error messages in hook handler spawner\r\n- [x] Trying to terminate server first, then killing, waiting for exit. Should prevent leaking server process.\r\n- [x] All spawn, wait and kill variables in constants for future configuration from CLI (#328, #235)\r\n- [x] Gracefully ending the server event though Dredd experiences exception (#330)\r\n- [x] Flaky tests workaround, hopefully. (#282)\r\n- [x] Tested against each hook handler test suites (gets enabled by https://github.com/apiaryio/dredd/pull/359)\r\n- [x] Removed stack tracing for non-exceptions in CLI\r\n- [x] 2x faster test-suite\r\n\r\n- [x] Rebased into one commit\r\n- [x] Rebased against master\r\n\r\nbefore:\r\n```\r\n  734 passing (4m)\r\n\r\n\r\nreal    4m20.975s\r\nuser    2m24.796s\r\nsys     0m5.835s\r\n```\r\n\r\nafter:\r\n```\r\n  820 passing (2m)\r\n\r\n\r\nreal\t2m15.007s\r\nuser\t0m43.701s\r\nsys     0m5.653s\r\n```"},"comment":{"url":"https://api.github.com/repos/apiaryio/dredd/issues/comments/187716258","html_url":"https://github.com/apiaryio/dredd/pull/332#issuecomment-187716258","issue_url":"https://api.github.com/repos/apiaryio/dredd/issues/332","id":187716258,"user":{"login":"honzajavorek","id":283441,"avatar_url":"https://avatars.githubusercontent.com/u/283441?v=3","gravatar_id":"","url":"https://api.github.com/users/honzajavorek","html_url":"https://github.com/honzajavorek","followers_url":"https://api.github.com/users/honzajavorek/followers","following_url":"https://api.github.com/users/honzajavorek/following{/other_user}","gists_url":"https://api.github.com/users/honzajavorek/gists{/gist_id}","starred_url":"https://api.github.com/users/honzajavorek/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/honzajavorek/subscriptions","organizations_url":"https://api.github.com/users/honzajavorek/orgs","repos_url":"https://api.github.com/users/honzajavorek/repos","events_url":"https://api.github.com/users/honzajavorek/events{/privacy}","received_events_url":"https://api.github.com/users/honzajavorek/received_events","type":"User","site_admin":false},"created_at":"2016-02-23T14:18:43Z","updated_at":"2016-02-23T14:18:43Z","body":"So after all, there are three outstanding issues:\r\n\r\n-  **Clarification of https://github.com/apiaryio/dredd/commit/552e13bf9c9834bf7e29d928fc74ed85bece312e#commitcomment-15883870**\r\n\r\n   I discussed it slightly with @netmilk and it looks like it's an intention. I've added comments noting the fact in 9de0f0e.\r\n\r\n-  **Why has coverage dropped? (Is https://github.com/apiaryio/dredd/pull/332#issuecomment-167741430 the answer?)**\r\n\r\n    I investigated coverage reports and there should be no problems with picking up the CLI tests. However, the CLI tests seem to be somewhat incomplete - the biggest gap is this:\r\n\r\n     <img width=\"1180\" alt=\"screen shot 2016-02-23 at 15 04 23\" src=\"https://cloud.githubusercontent.com/assets/283441/13253900/e50fc324-da3e-11e5-96ea-463bad592e00.png\">\r\n\r\n-  **The problems above were not caught by Dredd's own unit/integration tests... (e.g. [this one](https://github.com/apiaryio/dredd/blob/netmilk/proxyquireless-hooks-handler-client/test/unit/hooks-worker-client-test.coffee#L91))**\r\n\r\n    Related to previous point. Since the CLI tests are not as thorough as they look like, they probably missed some of the behavior which was then discovered by the cucumber integration tests performed individually with each hook handler implementation.\r\n\r\n### Verdict\r\n\r\nI refuse to continue with snowballing of this PR. The implementation looks all right and passes all tests we came up with and it is also manually tested by myself. Evidently, there is a gap in coverage/CLI tests. I'm suggesting that we merge this PR and revisit quality of the CLI tests in small, subsequent PR."}}