{"action":"created","issue":{"url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8396","repository_url":"https://api.github.com/repos/RobotLocomotion/drake","labels_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8396/labels{/name}","comments_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8396/comments","events_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8396/events","html_url":"https://github.com/RobotLocomotion/drake/pull/8396","id":306297764,"number":8396,"title":"Promote DRAKE_EXPECT_ERROR_MESSAGE et al. from geometry to common","user":{"login":"sherm1","id":4088016,"avatar_url":"https://avatars1.githubusercontent.com/u/4088016?v=4","gravatar_id":"","url":"https://api.github.com/users/sherm1","html_url":"https://github.com/sherm1","followers_url":"https://api.github.com/users/sherm1/followers","following_url":"https://api.github.com/users/sherm1/following{/other_user}","gists_url":"https://api.github.com/users/sherm1/gists{/gist_id}","starred_url":"https://api.github.com/users/sherm1/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sherm1/subscriptions","organizations_url":"https://api.github.com/users/sherm1/orgs","repos_url":"https://api.github.com/users/sherm1/repos","events_url":"https://api.github.com/users/sherm1/events{/privacy}","received_events_url":"https://api.github.com/users/sherm1/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"EricCousineau-TRI","id":26719449,"avatar_url":"https://avatars1.githubusercontent.com/u/26719449?v=4","gravatar_id":"","url":"https://api.github.com/users/EricCousineau-TRI","html_url":"https://github.com/EricCousineau-TRI","followers_url":"https://api.github.com/users/EricCousineau-TRI/followers","following_url":"https://api.github.com/users/EricCousineau-TRI/following{/other_user}","gists_url":"https://api.github.com/users/EricCousineau-TRI/gists{/gist_id}","starred_url":"https://api.github.com/users/EricCousineau-TRI/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/EricCousineau-TRI/subscriptions","organizations_url":"https://api.github.com/users/EricCousineau-TRI/orgs","repos_url":"https://api.github.com/users/EricCousineau-TRI/repos","events_url":"https://api.github.com/users/EricCousineau-TRI/events{/privacy}","received_events_url":"https://api.github.com/users/EricCousineau-TRI/received_events","type":"User","site_admin":false},"assignees":[{"login":"SeanCurtis-TRI","id":22159799,"avatar_url":"https://avatars3.githubusercontent.com/u/22159799?v=4","gravatar_id":"","url":"https://api.github.com/users/SeanCurtis-TRI","html_url":"https://github.com/SeanCurtis-TRI","followers_url":"https://api.github.com/users/SeanCurtis-TRI/followers","following_url":"https://api.github.com/users/SeanCurtis-TRI/following{/other_user}","gists_url":"https://api.github.com/users/SeanCurtis-TRI/gists{/gist_id}","starred_url":"https://api.github.com/users/SeanCurtis-TRI/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/SeanCurtis-TRI/subscriptions","organizations_url":"https://api.github.com/users/SeanCurtis-TRI/orgs","repos_url":"https://api.github.com/users/SeanCurtis-TRI/repos","events_url":"https://api.github.com/users/SeanCurtis-TRI/events{/privacy}","received_events_url":"https://api.github.com/users/SeanCurtis-TRI/received_events","type":"User","site_admin":false},{"login":"EricCousineau-TRI","id":26719449,"avatar_url":"https://avatars1.githubusercontent.com/u/26719449?v=4","gravatar_id":"","url":"https://api.github.com/users/EricCousineau-TRI","html_url":"https://github.com/EricCousineau-TRI","followers_url":"https://api.github.com/users/EricCousineau-TRI/followers","following_url":"https://api.github.com/users/EricCousineau-TRI/following{/other_user}","gists_url":"https://api.github.com/users/EricCousineau-TRI/gists{/gist_id}","starred_url":"https://api.github.com/users/EricCousineau-TRI/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/EricCousineau-TRI/subscriptions","organizations_url":"https://api.github.com/users/EricCousineau-TRI/orgs","repos_url":"https://api.github.com/users/EricCousineau-TRI/repos","events_url":"https://api.github.com/users/EricCousineau-TRI/events{/privacy}","received_events_url":"https://api.github.com/users/EricCousineau-TRI/received_events","type":"User","site_admin":false}],"milestone":null,"comments":9,"created_at":"2018-03-18T23:45:22Z","updated_at":"2018-03-19T17:53:24Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/RobotLocomotion/drake/pulls/8396","html_url":"https://github.com/RobotLocomotion/drake/pull/8396","diff_url":"https://github.com/RobotLocomotion/drake/pull/8396.diff","patch_url":"https://github.com/RobotLocomotion/drake/pull/8396.patch"},"body":"This PR replaces an assortment of bespoke macros for verifying in unit tests that an error message contains the expected text. Call sites are updated to use the now-standardized signatures. \r\n\r\nSemantics of the \"IS_ARMED\" forms has changed: as before the message is only _required_ in Debug\r\n(actually `DRAKE_ENABLE_ASSERTS`), but is now _optional_ in Release (previously it was _forbidden_ in Release). _If_ the Release build actually throws, the error message must meet the same requirements as in Debug.\r\n\r\nPrompted by [reviewer request](https://reviewable.io/reviews/robotlocomotion/drake/8372#-L7o6Anm1opaM6EjAmVz) in #8372.\r\n\r\nI am not adding standalone unit tests for these macros but they are tested extensively by the many unit tests in which they are used.\r\n\r\nResolves #8003.\r\n\r\n<!-- Reviewable:start -->\r\n---\r\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/robotlocomotion/drake/8396)\r\n<!-- Reviewable:end -->\r\n"},"comment":{"url":"https://api.github.com/repos/RobotLocomotion/drake/issues/comments/374305915","html_url":"https://github.com/RobotLocomotion/drake/pull/8396#issuecomment-374305915","issue_url":"https://api.github.com/repos/RobotLocomotion/drake/issues/8396","id":374305915,"user":{"login":"sherm1","id":4088016,"avatar_url":"https://avatars1.githubusercontent.com/u/4088016?v=4","gravatar_id":"","url":"https://api.github.com/users/sherm1","html_url":"https://github.com/sherm1","followers_url":"https://api.github.com/users/sherm1/followers","following_url":"https://api.github.com/users/sherm1/following{/other_user}","gists_url":"https://api.github.com/users/sherm1/gists{/gist_id}","starred_url":"https://api.github.com/users/sherm1/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sherm1/subscriptions","organizations_url":"https://api.github.com/users/sherm1/orgs","repos_url":"https://api.github.com/users/sherm1/repos","events_url":"https://api.github.com/users/sherm1/events{/privacy}","received_events_url":"https://api.github.com/users/sherm1/received_events","type":"User","site_admin":false},"created_at":"2018-03-19T17:53:23Z","updated_at":"2018-03-19T17:53:23Z","author_association":"MEMBER","body":"Added good doxygen, renamed macros, per f2f with @seancurtis-tri changed semantics of the \"IS_ARMED\" versions to allow Release builds to _optionally_ throw, but if they do it still has to be the right message. I added a TODO regarding unit tests but that's too much for me to tackle now. This PR is a substantial improvement over the status quo already. If anyone else wants to attempt those tricky tests, please do! (In a separate PR) \n\nI substantially re-engineered the macros in the header to avoid duplication -- ptal.\n\n---\n\nReview status: 9 of 19 files reviewed at latest revision, 4 unresolved discussions.\n\n---\n\n*[a discussion](https://reviewable.io:443/reviews/robotlocomotion/drake/8396#-L7yPBsa5Lo012JIx0nj:-L7zMWxBISpqyZM-3GJQ:bfdmcx0) (no related file):*\n<details><summary><i>Previously, amcastro-tri (Alejandro Castro) wrote…</i></summary><blockquote>\n\nBTW. Probably it'd be a good idea to mirror `drake_asserth.h` regarding how to document these with Doxygen.\n</blockquote></details>\n\nDone, ptal.\n\n---\n\n*[common/test/type_safe_index_test.cc, line 44 at r1](https://reviewable.io:443/reviews/robotlocomotion/drake/8396#-L7yhFbao10_oTEBgNZ0:-L7zNR91m4WjSWDqUCYT:b-jsrqj9) ([raw file](https://github.com/robotlocomotion/drake/blob/ae06269fda520ce45345d48b181c6d2663fc3fbd/common/test/type_safe_index_test.cc#L44)):*\n<details><summary><i>Previously, EricCousineau-TRI (Eric Cousineau) wrote…</i></summary><blockquote>\n\nGiven that this is specifically for catching exception, can we mirror the naming for `EXPECT_THROWS` / `ASSERT_THROWS` and rename this to `DRAKE_EXPECT_THROWS_MESSAGE*``?\n</blockquote></details>\n\nDone. Great idea.\n\n---\n\n*[common/test_utilities/expect_error_message.h, line 46 at r1](https://reviewable.io:443/reviews/robotlocomotion/drake/8396#-L7yiDOwwK4Y1gj5MR2c:-L7zNUrw8Cli4j6ENSvn:b-8gpzim) ([raw file](https://github.com/robotlocomotion/drake/blob/ae06269fda520ce45345d48b181c6d2663fc3fbd/common/test_utilities/expect_error_message.h#L46)):*\n<details><summary><i>Previously, EricCousineau-TRI (Eric Cousineau) wrote…</i></summary><blockquote>\n\nhttps://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuide.md#catching-failures\n</blockquote></details>\n\nAdded a TODO but I don't want to attempt these tricky tests. This PR is a side project to improve the status quo which I think it has done -- I want to get back to the caching PR. Love to see someone add unit tests to improve it further, in a separate PR please.\n\n---\n\n*[geometry/BUILD.bazel, line 195 at r1](https://reviewable.io:443/reviews/robotlocomotion/drake/8396#-L7yPsaHXwKkvfc4GeAd:-L7zO-P6UE3gzv97DRVV:b-896fix) ([raw file](https://github.com/robotlocomotion/drake/blob/ae06269fda520ce45345d48b181c6d2663fc3fbd/geometry/BUILD.bazel#L195)):*\n<details><summary><i>Previously, amcastro-tri (Alejandro Castro) wrote…</i></summary><blockquote>\n\nBTW maybe `common/test_utilities:expect_error_message` alone is enough.\n</blockquote></details>\n\nDone.\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/robotlocomotion/drake/8396#-:-L7zMavDScWmEfRsE8hN:b-jzsoga)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"sherm1"}}