{"action":"created","issue":{"url":"https://api.github.com/repos/Snaipe/Criterion/issues/165","repository_url":"https://api.github.com/repos/Snaipe/Criterion","labels_url":"https://api.github.com/repos/Snaipe/Criterion/issues/165/labels{/name}","comments_url":"https://api.github.com/repos/Snaipe/Criterion/issues/165/comments","events_url":"https://api.github.com/repos/Snaipe/Criterion/issues/165/events","html_url":"https://github.com/Snaipe/Criterion/issues/165","id":178138411,"number":165,"title":"Rewrite the assertion API","user":{"login":"Snaipe","id":977973,"avatar_url":"https://avatars.githubusercontent.com/u/977973?v=3","gravatar_id":"","url":"https://api.github.com/users/Snaipe","html_url":"https://github.com/Snaipe","followers_url":"https://api.github.com/users/Snaipe/followers","following_url":"https://api.github.com/users/Snaipe/following{/other_user}","gists_url":"https://api.github.com/users/Snaipe/gists{/gist_id}","starred_url":"https://api.github.com/users/Snaipe/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Snaipe/subscriptions","organizations_url":"https://api.github.com/users/Snaipe/orgs","repos_url":"https://api.github.com/users/Snaipe/repos","events_url":"https://api.github.com/users/Snaipe/events{/privacy}","received_events_url":"https://api.github.com/users/Snaipe/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/Snaipe/Criterion/labels/API","name":"API","color":"d4c5f9"},{"url":"https://api.github.com/repos/Snaipe/Criterion/labels/Enhancement","name":"Enhancement","color":"006b75"},{"url":"https://api.github.com/repos/Snaipe/Criterion/labels/Normal","name":"Normal","color":"fff875"}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/Snaipe/Criterion/milestones/3","html_url":"https://github.com/Snaipe/Criterion/milestones/v3.0.0","labels_url":"https://api.github.com/repos/Snaipe/Criterion/milestones/3/labels","id":1717182,"number":3,"title":"v3.0.0","description":"","creator":{"login":"Snaipe","id":977973,"avatar_url":"https://avatars.githubusercontent.com/u/977973?v=3","gravatar_id":"","url":"https://api.github.com/users/Snaipe","html_url":"https://github.com/Snaipe","followers_url":"https://api.github.com/users/Snaipe/followers","following_url":"https://api.github.com/users/Snaipe/following{/other_user}","gists_url":"https://api.github.com/users/Snaipe/gists{/gist_id}","starred_url":"https://api.github.com/users/Snaipe/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Snaipe/subscriptions","organizations_url":"https://api.github.com/users/Snaipe/orgs","repos_url":"https://api.github.com/users/Snaipe/repos","events_url":"https://api.github.com/users/Snaipe/events{/privacy}","received_events_url":"https://api.github.com/users/Snaipe/received_events","type":"User","site_admin":false},"open_issues":5,"closed_issues":1,"state":"open","created_at":"2016-04-19T14:57:39Z","updated_at":"2016-09-20T18:29:05Z","due_on":null,"closed_at":null},"comments":1,"created_at":"2016-09-20T18:29:05Z","updated_at":"2016-09-20T21:47:26Z","closed_at":null,"body":"The assertion API is a bit clunky as it is. I think that the design of that part of the API needs to change for the better for 3.0, as we've learned quite some things in the past two years.\r\n\r\nMy biggest gripe with the current design is that it's at the same time too inconsistent and too shallow to be useful for both the user and the runtime (it makes printing actual/expected diffs hard).\r\n\r\nI have three proposals for a redesign in mind:\r\n\r\n## Proposal 1: use long names and per-type functions:\r\n\r\n```c\r\nTest(assert, full)\r\n{\r\n    /* cr_assert(Cond, [msg, ...]) */\r\n    cr_assert(1);\r\n    cr_assert(1, \"Hello %s\", \"World!\");\r\n\r\n    /* cr_assert_<cmp>_<type>(A, B, [msg, ...])\r\n     *\r\n     * comparators: eq, ne, lt, le, gt, ge\r\n     * types: i8, i16, i32, i64, u8, u16, u32, u64,\r\n     *        str, wcs, tcs (windows-specific TCHAR)\r\n     *        uchar, schar, uint, sint, ulong, slong,\r\n     *        ullong, sllong, flt, dbl, ldbl, bool\r\n     */\r\n    cr_assert_eq_i8(1, 1);\r\n\r\n    /* \"cr_assert_<cmp>\" will simply use the relevant operator and\r\n     * an appropriate stringification for compilers supporting it. */\r\n    cr_assert_eq(1, 1);\r\n\r\n    /* the \"mem\" type requires an additional size parameter:\r\n     * cr_assert_<cmp>_mem(A, B, Size, [msg, ...])\r\n     */\r\n    cr_assert_eq_mem(&(int){ 1 }, &(int){ 1 }, sizeof (int));\r\n\r\n    /* cr_assert_<cmp>_cmp(&A, &B, Cmp, [msg, ...])\r\n     * Must take pointers, and uses a user-provided comparator.\r\n     *\r\n     * Unnecessary on C++ compilers where operator== is used.\r\n     */\r\n    cr_assert_eq_cmp(&(int){ 1 }, &(int){ 1 }, int_cmp);\r\n\r\n    /* pointer checks */\r\n    cr_assert_null(NULL);\r\n    cr_assert_notnull(0x1);\r\n\r\n    /* C++ exceptions */\r\n    cr_assert_throw({ throw std::exception(\"test\") });\r\n    cr_assert_nothrow({});\r\n    cr_assert_anythrow({ throw std::exception(\"test\") });\r\n}\r\n```\r\n\r\nThis one is the least risky: we basically take the current API, add some type-specific macros, and make the naming and usage conventions consistent. Nothing too far-fetched, and quite consistent with the C philosophy.\r\n\r\n## Proposal 2: cleaner syntax w/ assertion sub-expressions:\r\n\r\n```c\r\nTest(assert, nested)\r\n{\r\n    /* cr_assert(Cond, [msg, ...]) */\r\n    cr_assert(1);\r\n    cr_assert(1, \"Hello %s\", \"World!\");\r\n\r\n    /* negation */\r\n    cr_assert(not(0));\r\n\r\n    /* comparison\r\n     *\r\n     * cr_assert(<cmp>(A, B, <type_tag>), [msg, ...]) \r\n     *\r\n     * comparators: eq, ne, lt, le, gt, ge\r\n     * types tags: i8, i16, i32, i64, u8, u16, u32, u64,\r\n     *        str, wcs, tcs (windows-specific TCHAR), mem\r\n     *        uchar, schar, uint, sint, ulong, slong,\r\n     *        ullong, sllong, flt, dbl, ldbl, bool\r\n     */\r\n    cr_assert(eq(1, 1, i8));\r\n\r\n    /* mem uses the struct cr_mem type */\r\n    struct cr_mem a = { &(int){ 1 }, sizeof (int) };\r\n    struct cr_mem b = { &(int){ 1 }, sizeof (int) };\r\n    cr_assert(eq(a, b, mem));\r\n\r\n    /* mem<cmp>(arr1, arr2, size) is a syntactic sugar for arrays\r\n     * that have the same size */\r\n    cr_assert(memeq(&(int){ 1 }, &(int){ 1 }, sizeof (int)));\r\n\r\n    /* on supported compilers, the type tag can be dropped entirely */\r\n    cr_assert(eq(1, 1));\r\n\r\n    /* on C compilers, using a custom type tag allows the usage of\r\n     * custom comparators and stringification functions.\r\n     *\r\n     * on C++ compilers, this is unnecessary as operator== and operator<<\r\n     * are used.\r\n     */\r\n    cr_assert(eq(1, 1, my_type_id));\r\n\r\n    /* composition */\r\n    cr_assert(or(1, 0)) /* expr1 || expr2 */\r\n    cr_assert(and(1, 1)) /* expr1 && expr2 */\r\n    cr_assert(any(0, 0, 1, 0)); /* at least one expr must be true */\r\n    cr_assert(none(0, 0, 0, 0)); /* all exprs must be false */\r\n    cr_assert(all(1, 1, 1, 1)); /* all exprs must be true */\r\n\r\n    cr_assert(or(and(eq(1, 1), eq(2, 2)), eq(1, 2)));\r\n\r\n    /* pointer checks */\r\n    cr_assert(null(NULL));\r\n\r\n    /* C++ exceptions */\r\n    cr_assert(throw({ throw std::exception(\"test\"); }));\r\n    cr_assert(nothrow({}));\r\n    cr_assert(anythrow({ throw std::exception(\"test\"); }));\r\n}\r\n```\r\n\r\nI like this one quite a lot. Here, we only need to define two macros, cr_assert and cr_expect, fitted with some token recognition for eq, ne, ... and the rest.\r\n\r\nThe usage of type tags makes it possible for us to define easily comparision functions and stringification functions that would make possible generic value diffing as described in #105, and best of all would allow users to implement for instance `cr_user_<tag>_cmp` and `cr_user_<tag>_tostr` that would be then picked up as possible comparison and stringification functions for their cursom type tag.\r\n\r\nIt is, however, quite unclear if this design is at all possible and will require some experimentation, but my experience with mimick tells me that this should definitely be implementable.\r\n\r\n## Proposal 3: object-oriented-like composition:\r\n\r\n```c\r\nTest(assert, compound)\r\n{\r\n    cr_assert().istrue(1).done();\r\n    cr_assert().istrue(1).msg(\"Hello %s\", \"World!\").done();\r\n\r\n    /* negation */\r\n    cr_assert().not().istrue(1).done();\r\n    cr_assert().isfalse(0).done(); /* equivalent */\r\n\r\n    /* comparison (needs pointers & comparator + stringification functions) */\r\n    cr_assert().eq(&(int){ 1 }, &(int){ 1 }, cr_sint).done();\r\n\r\n    /* mem uses the struct cr_mem type */\r\n    struct cr_mem a = { &(int){ 1 }, sizeof (int) };\r\n    struct cr_mem b = { &(int){ 1 }, sizeof (int) };\r\n    cr_assert().eq(a, b, cr_mem).done();\r\n\r\n    /* user types needs comparator and stringification function */\r\n    struct cr_user_type my_type = { .cmp = my_cmp, .str = my_str };\r\n    cr_assert().eq(A, B, my_type).done();\r\n\r\n    /* compound expressions */\r\n    cr_assert()\r\n        .eq(A, B, my_type).or()\r\n        .lt(A, B, my_type).and()\r\n        .gt(A, B, my_type)\r\n        .done();\r\n\r\n    /* pointer checks */\r\n    cr_assert().null(NULL).done();\r\n\r\n}\r\n```\r\n\r\nThis one is more for posterity, as one of the \"should be possible\" designs, but while the compound expressions and chaining coupled with the external `.msg()` are nice, I have a few problems with the trailing `.done()` and the lack of automatic type handling (you always have to provide a comparator here).\r\n\r\nThe idea of chaining a `.msg` could still be interesting but would require deep changes in the protocol."},"comment":{"url":"https://api.github.com/repos/Snaipe/Criterion/issues/comments/248444920","html_url":"https://github.com/Snaipe/Criterion/issues/165#issuecomment-248444920","issue_url":"https://api.github.com/repos/Snaipe/Criterion/issues/165","id":248444920,"user":{"login":"am11","id":3840695,"avatar_url":"https://avatars.githubusercontent.com/u/3840695?v=3","gravatar_id":"","url":"https://api.github.com/users/am11","html_url":"https://github.com/am11","followers_url":"https://api.github.com/users/am11/followers","following_url":"https://api.github.com/users/am11/following{/other_user}","gists_url":"https://api.github.com/users/am11/gists{/gist_id}","starred_url":"https://api.github.com/users/am11/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/am11/subscriptions","organizations_url":"https://api.github.com/users/am11/orgs","repos_url":"https://api.github.com/users/am11/repos","events_url":"https://api.github.com/users/am11/events{/privacy}","received_events_url":"https://api.github.com/users/am11/received_events","type":"User","site_admin":false},"created_at":"2016-09-20T21:47:26Z","updated_at":"2016-09-20T21:47:26Z","body":"+1 for proposal 3.\r\n\r\nWould be nice if `.true` and `.false` are preferred over `.istrue` and `.isfalse` respectively.\r\n\r\nAlso, slightly better semantics and if possible, `done()` should have a once-per-test constraint ([source of inspiration](https://github.com/mochajs/mocha/blob/9e7f5f207a9905d3243d33a22c81d3b0c960ccda/test/browser/multiple-done.spec.js)).\r\n"}}
{"actor":{"display_login":"am11"}}