{"action":"created","issue":{"url":"https://api.github.com/repos/unittest-cpp/unittest-cpp/issues/125","repository_url":"https://api.github.com/repos/unittest-cpp/unittest-cpp","labels_url":"https://api.github.com/repos/unittest-cpp/unittest-cpp/issues/125/labels{/name}","comments_url":"https://api.github.com/repos/unittest-cpp/unittest-cpp/issues/125/comments","events_url":"https://api.github.com/repos/unittest-cpp/unittest-cpp/issues/125/events","html_url":"https://github.com/unittest-cpp/unittest-cpp/issues/125","id":173480717,"number":125,"title":"Parameterized tests, parameterized suites (parameters for tests)","user":{"login":"killoctal","id":2680908,"avatar_url":"https://avatars.githubusercontent.com/u/2680908?v=3","gravatar_id":"","url":"https://api.github.com/users/killoctal","html_url":"https://github.com/killoctal","followers_url":"https://api.github.com/users/killoctal/followers","following_url":"https://api.github.com/users/killoctal/following{/other_user}","gists_url":"https://api.github.com/users/killoctal/gists{/gist_id}","starred_url":"https://api.github.com/users/killoctal/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/killoctal/subscriptions","organizations_url":"https://api.github.com/users/killoctal/orgs","repos_url":"https://api.github.com/users/killoctal/repos","events_url":"https://api.github.com/users/killoctal/events{/privacy}","received_events_url":"https://api.github.com/users/killoctal/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/unittest-cpp/unittest-cpp/labels/discussion","name":"discussion","color":"009800"},{"url":"https://api.github.com/repos/unittest-cpp/unittest-cpp/labels/enhancement","name":"enhancement","color":"0052cc"}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":11,"created_at":"2016-08-26T15:30:48Z","updated_at":"2016-09-01T13:28:38Z","closed_at":null,"body":"As discussed in the pull request [#124](https://github.com/unittest-cpp/unittest-cpp/pull/124), I proposed my implementation of parameterized tests but the PR was rejected. This means I have to improve ;)\r\n\r\nFor me the usage is really simple and convenient, you have some examples in the [tests file](https://github.com/killoctal/unittest-cpp/blob/17d6e82ca66db64d4c93e465c7f631d6c47e27aa/tests/TestParameterizedTest.cpp) but everyone has it own point of view so please feel free to make suggestions or give ideas to make it better.\r\n\r\nThe goal is to make the feature to be accepted :)"},"comment":{"url":"https://api.github.com/repos/unittest-cpp/unittest-cpp/issues/comments/244078282","html_url":"https://github.com/unittest-cpp/unittest-cpp/issues/125#issuecomment-244078282","issue_url":"https://api.github.com/repos/unittest-cpp/unittest-cpp/issues/125","id":244078282,"user":{"login":"pjohnmeyer","id":122016,"avatar_url":"https://avatars.githubusercontent.com/u/122016?v=3","gravatar_id":"","url":"https://api.github.com/users/pjohnmeyer","html_url":"https://github.com/pjohnmeyer","followers_url":"https://api.github.com/users/pjohnmeyer/followers","following_url":"https://api.github.com/users/pjohnmeyer/following{/other_user}","gists_url":"https://api.github.com/users/pjohnmeyer/gists{/gist_id}","starred_url":"https://api.github.com/users/pjohnmeyer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pjohnmeyer/subscriptions","organizations_url":"https://api.github.com/users/pjohnmeyer/orgs","repos_url":"https://api.github.com/users/pjohnmeyer/repos","events_url":"https://api.github.com/users/pjohnmeyer/events{/privacy}","received_events_url":"https://api.github.com/users/pjohnmeyer/received_events","type":"User","site_admin":false},"created_at":"2016-09-01T13:28:38Z","updated_at":"2016-09-01T13:28:38Z","body":"Having thought about this and the examples above more, my three pain points stand, and I'll add a 3b from @killoctal's notes and a 4th of my own.\r\n\r\n1. Repeating the `TEST_FIXTURE(TestData,` part over and over is a bit obnoxious\r\n2. Error messages aren't very good in this case.\r\n3. The odds and evens vectors are either:\r\n    a. constructed for every test, or\r\n    b. subject to \"painful global initializations\"\r\n4. The developer has to \"manually\" write loops in each test\r\n\r\nIn this comment I aim to address all of these issues, having spent more time mulling over them.\r\n\r\nItems 1 and 3a are addressed by, in cases such as this one in which the same test data is used over and over, by just not doing it that way. This leaves us with 2 and 3b.\r\n\r\nPersonally, I just don't find these \"painful global initializations\" that painful. To take my previous example and address this, I would do something like this:\r\n\r\n```cpp\r\n   namespace\r\n   {\r\n      const std::vector<int>& odds()\r\n      {\r\n         static const std::vector<int> values = {1,3,5};\r\n         return values;\r\n      }\r\n\r\n      const std::vector<int>& evens()\r\n      {\r\n         static const std::vector<int> values = {2,4,6};\r\n         return values;\r\n      }\r\n   }\r\n\r\n   bool is_odd(int x) { return ((x & 1) == 1); }\r\n   bool is_positive(int x) { return (x > 0); }\r\n\r\n   TEST(OddDetection)\r\n   {\r\n      for (size_t idx = 0; idx < odds().size(); ++idx)\r\n      {\r\n         CHECK(is_odd(odds()[idx]));\r\n      }\r\n   } \r\n   // ...\r\n   // Imagine more tests re evens, etc.\r\n   // ...\r\n   TEST(PositiveDetection)\r\n   {\r\n      for (size_t idx = 0; idx < odds().size(); ++idx)\r\n      {\r\n         CHECK(is_positive(odds()[idx]));\r\n      }\r\n\r\n      for (size_t idx = 0; idx < evens().size(); ++idx)\r\n      {\r\n         CHECK(is_positive(evens()[idx]));\r\n      }\r\n   }\r\n```\r\n\r\nNow yes, the developer has to write for loops to check all inputs (issue 4 above) -- but at least that's a basic skill, and it's immediately obvious what is happening to the reader.\r\n\r\nLets turn then to issue 2. If we create an implementation of custom messages, we can get rid of this. Imagine for a moment that you can do something like this:\r\n\r\n```cpp\r\n   int val_under_test = odds()[idx];\r\n   CHECK(is_positive(val_under_test)) << \" value is \" << val_under_test;\r\n```\r\n\r\nand that in our aforementioned failure case, you now saw this:\r\n\r\n```\r\n  C:\\Users\\pjohn\\code\\pjohnmeyer\\unittest-cpp\\tests\\TestCheckMacros.cpp(548): error : Failure in PositiveDetection: is_positive(val_under_test); value is -1 [C:\\Users\\pjohn\\code\\pjohnmeyer\\unittest-cpp\\builds\\vs2015\\TestUnitTest++.vcxproj]\r\n  C:\\Users\\pjohn\\code\\pjohnmeyer\\unittest-cpp\\tests\\TestCheckMacros.cpp(548): error : Failure in PositiveDetection: is_positive(odds_and_evens[idx]); value is -3 [C:\\Users\\pjohn\\code\\pjohnmeyer\\unittest-cpp\\builds\\vs2015\\TestUnitTest++.vcxproj]\r\n  C:\\Users\\pjohn\\code\\pjohnmeyer\\unittest-cpp\\tests\\TestCheckMacros.cpp(548): error : Failure in PositiveDetection: is_positive(odds_and_evens[idx]); value is -5 [C:\\Users\\pjohn\\code\\pjohnmeyer\\unittest-cpp\\builds\\vs2015\\TestUnitTest++.vcxproj]\r\n```\r\n\r\nNow it's incredibly clear what is happening. It's a little gross here that we're duplicating `val_under_test`, but remember this is an academic example. In a more complex example, the user can output the _most meaningful thing to them_ for that particular `CHECK` without outputting the entire parameter set. This is arguably a good duplication because it's giving the user full control of the output.\r\n\r\nFor my money, I think the bang for the buck is in supporting custom messages over parameterized testing. But, I definitely see the value of being able to wrap a set of tests without having to repeat the loops.\r\n\r\nThe thing that I just can't get past about your proposed implementation is this: it doesn't look like C++. It's a _cool_ implementation -- call these functions and you implicitly get test repetition. But \"normal\" C++ doesn't behave like this. Looping is explicit. Wrapping the TEST in a `PARAMETERIZED` block helps, but it still, in my opinion, hides too much.\r\n\r\nI'm going to see if I have time to spend on an alternate prototype implementation or two to throw out there, but if somebody else wants to tackle this I'd love to see others' ideas. I'm sure somebody out there has done something like this for their project(s).\r\n\r\n_P.S. I've decided I don't like the name `USING` because, while clearly a macro, it's the same word as the C++ keyword `using`._"}}
{"actor":{"display_login":"pjohnmeyer"}}