From cc172642702e59637ee237317c0d6fab903d176f Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Fri, 17 Feb 2017 12:10:19 +0000 Subject: [PATCH 1/3] p_snprintf: no need for arguments to a format `snprintf` requires a _format_ but does not require _arguments_ to the format. eg: `snprintf(buf, 42, "hi")` is perfectly legal. Expand the macro to match. Without this, `p_sprintf(buf, 42, "hi")` errors with: ``` error: expected expression p_snprintf(msg, 42, "hi"); ^ src/unix/posix.h:53:34: note: expanded from macro 'p_snprintf' ^ /usr/include/secure/_stdio.h:57:73: note: expanded from macro 'snprintf' __builtin___snprintf_chk (str, len, 0, __darwin_obsz(str), __VA_ARGS__) ``` --- src/unix/posix.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/unix/posix.h b/src/unix/posix.h index ad13291e8..b4786403f 100644 --- a/src/unix/posix.h +++ b/src/unix/posix.h @@ -50,7 +50,7 @@ extern char *p_realpath(const char *, char *); #define p_strcasecmp(s1, s2) strcasecmp(s1, s2) #define p_strncasecmp(s1, s2, c) strncasecmp(s1, s2, c) #define p_vsnprintf(b, c, f, a) vsnprintf(b, c, f, a) -#define p_snprintf(b, c, f, ...) snprintf(b, c, f, __VA_ARGS__) +#define p_snprintf(b, c, ...) snprintf(b, c, __VA_ARGS__) #define p_mkstemp(p) mkstemp(p) #define p_chdir(p) chdir(p) #define p_chmod(p,m) chmod(p, m) From a1dcc83030f9073fe29054ecaf7647dca1fec2ec Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Fri, 17 Feb 2017 12:13:35 +0000 Subject: [PATCH 2/3] tests: provide better pass/failure error messages Provide more detailed messages when conditions pass or fail unexpectedly. In particular, this provides the error messages when a test fails with a different error code than was expected. --- tests/attr/ignore.c | 4 ++-- tests/clar_libgit2.c | 14 +++++++++++--- tests/clar_libgit2.h | 22 +++++++++++++--------- tests/repo/env.c | 12 ++++++------ tests/status/ignore.c | 4 ++-- tests/submodule/submodule_helpers.c | 2 +- 6 files changed, 35 insertions(+), 23 deletions(-) diff --git a/tests/attr/ignore.c b/tests/attr/ignore.c index f1fe1c71f..a8845e463 100644 --- a/tests/attr/ignore.c +++ b/tests/attr/ignore.c @@ -21,8 +21,8 @@ static void assert_is_ignored_( { int is_ignored = 0; - cl_git_pass_( - git_ignore_path_is_ignored(&is_ignored, g_repo, filepath), file, line); + cl_git_exec( + git_ignore_path_is_ignored(&is_ignored, g_repo, filepath), 0, file, line); clar__assert_equal( file, line, "expected != is_ignored", 1, "%d", diff --git a/tests/clar_libgit2.c b/tests/clar_libgit2.c index 314d3441e..bd10c009d 100644 --- a/tests/clar_libgit2.c +++ b/tests/clar_libgit2.c @@ -4,12 +4,20 @@ #include "git2/sys/repository.h" void cl_git_report_failure( - int error, const char *file, int line, const char *fncall) + int error, int expected, const char *file, int line, const char *fncall) { char msg[4096]; const git_error *last = giterr_last(); - p_snprintf(msg, 4096, "error %d - %s", - error, last ? last->message : ""); + + if (expected) + p_snprintf(msg, 4096, "error %d (expected %d) - %s", + error, expected, last ? last->message : ""); + else if (error || last) + p_snprintf(msg, 4096, "error %d - %s", + error, last ? last->message : ""); + else + p_snprintf(msg, 4096, "no error, expected non-zero return"); + clar__assert(0, file, line, fncall, msg, 1); } diff --git a/tests/clar_libgit2.h b/tests/clar_libgit2.h index fc08bbf1f..1d77bc6d6 100644 --- a/tests/clar_libgit2.h +++ b/tests/clar_libgit2.h @@ -12,13 +12,15 @@ * * Use this wrapper around all `git_` library calls that return error codes! */ -#define cl_git_pass(expr) cl_git_pass_((expr), __FILE__, __LINE__) +#define cl_git_pass(expr) cl_git_exec((expr), 0, __FILE__, __LINE__) -#define cl_git_pass_(expr, file, line) do { \ +#define cl_git_fail_with(error, expr) cl_git_exec((expr), error, __FILE__, __LINE__) + +#define cl_git_exec(expr, expected, file, line) do { \ int _lg2_error; \ giterr_clear(); \ - if ((_lg2_error = (expr)) != 0) \ - cl_git_report_failure(_lg2_error, file, line, "Function call failed: " #expr); \ + if ((_lg2_error = (expr)) != expected) \ + cl_git_report_failure(_lg2_error, expected, file, line, "Function call failed: " #expr); \ } while (0) /** @@ -26,9 +28,11 @@ * just for consistency. Use with `git_` library * calls that are supposed to fail! */ -#define cl_git_fail(expr) cl_must_fail(expr) - -#define cl_git_fail_with(expr, error) cl_assert_equal_i(error,expr) +#define cl_git_fail(expr) do { \ + giterr_clear(); \ + if ((expr) == 0) \ + cl_git_report_failure(0, 0, __FILE__, __LINE__, "Function call succeeded: " #expr); \ + } while (0) /** * Like cl_git_pass, only for Win32 error code conventions @@ -37,7 +41,7 @@ int _win32_res; \ if ((_win32_res = (expr)) == 0) { \ giterr_set(GITERR_OS, "Returned: %d, system error code: %d", _win32_res, GetLastError()); \ - cl_git_report_failure(_win32_res, __FILE__, __LINE__, "System call failed: " #expr); \ + cl_git_report_failure(_win32_res, 0, __FILE__, __LINE__, "System call failed: " #expr); \ } \ } while(0) @@ -86,7 +90,7 @@ GIT_INLINE(void) cl_git_thread_check(void *data) clar__assert(0, threaderr->file, threaderr->line, threaderr->expr, threaderr->error_msg, 1); } -void cl_git_report_failure(int, const char *, int, const char *); +void cl_git_report_failure(int, int, const char *, int, const char *); #define cl_assert_at_line(expr,file,line) \ clar__assert((expr) != 0, file, line, "Expression is not true: " #expr, NULL, 1) diff --git a/tests/repo/env.c b/tests/repo/env.c index 5a89c0d49..235f787ab 100644 --- a/tests/repo/env.c +++ b/tests/repo/env.c @@ -56,8 +56,8 @@ static int GIT_FORMAT_PRINTF(2, 3) cl_setenv_printf(const char *name, const char static void env_pass_(const char *path, const char *file, int line) { git_repository *repo; - cl_git_pass_(git_repository_open_ext(NULL, path, GIT_REPOSITORY_OPEN_FROM_ENV, NULL), file, line); - cl_git_pass_(git_repository_open_ext(&repo, path, GIT_REPOSITORY_OPEN_FROM_ENV, NULL), file, line); + cl_git_exec(git_repository_open_ext(NULL, path, GIT_REPOSITORY_OPEN_FROM_ENV, NULL), 0, file, line); + cl_git_exec(git_repository_open_ext(&repo, path, GIT_REPOSITORY_OPEN_FROM_ENV, NULL), 0, file, line); cl_assert_at_line(git__suffixcmp(git_repository_path(repo), "attr/.git/") == 0, file, line); cl_assert_at_line(git__suffixcmp(git_repository_workdir(repo), "attr/") == 0, file, line); cl_assert_at_line(!git_repository_is_bare(repo), file, line); @@ -98,24 +98,24 @@ static void env_check_objects_(bool a, bool t, bool p, const char *file, int lin cl_git_pass(git_oid_fromstr(&oid_a, "45141a79a77842c59a63229403220a4e4be74e3d")); cl_git_pass(git_oid_fromstr(&oid_t, "1385f264afb75a56a5bec74243be9b367ba4ca08")); cl_git_pass(git_oid_fromstr(&oid_p, "0df1a5865c8abfc09f1f2182e6a31be550e99f07")); - cl_git_pass_(git_repository_open_ext(&repo, "attr", GIT_REPOSITORY_OPEN_FROM_ENV, NULL), file, line); + cl_git_exec(git_repository_open_ext(&repo, "attr", GIT_REPOSITORY_OPEN_FROM_ENV, NULL), 0, file, line); if (a) { - cl_git_pass_(git_object_lookup(&object, repo, &oid_a, GIT_OBJ_BLOB), file, line); + cl_git_exec(git_object_lookup(&object, repo, &oid_a, GIT_OBJ_BLOB), 0, file, line); git_object_free(object); } else { cl_git_fail_at_line(git_object_lookup(&object, repo, &oid_a, GIT_OBJ_BLOB), file, line); } if (t) { - cl_git_pass_(git_object_lookup(&object, repo, &oid_t, GIT_OBJ_BLOB), file, line); + cl_git_exec(git_object_lookup(&object, repo, &oid_t, GIT_OBJ_BLOB), 0, file, line); git_object_free(object); } else { cl_git_fail_at_line(git_object_lookup(&object, repo, &oid_t, GIT_OBJ_BLOB), file, line); } if (p) { - cl_git_pass_(git_object_lookup(&object, repo, &oid_p, GIT_OBJ_COMMIT), file, line); + cl_git_exec(git_object_lookup(&object, repo, &oid_p, GIT_OBJ_COMMIT), 0, file, line); git_object_free(object); } else { cl_git_fail_at_line(git_object_lookup(&object, repo, &oid_p, GIT_OBJ_COMMIT), file, line); diff --git a/tests/status/ignore.c b/tests/status/ignore.c index c4878b2dd..a9b96114b 100644 --- a/tests/status/ignore.c +++ b/tests/status/ignore.c @@ -20,8 +20,8 @@ static void assert_ignored_( bool expected, const char *filepath, const char *file, int line) { int is_ignored = 0; - cl_git_pass_( - git_status_should_ignore(&is_ignored, g_repo, filepath), file, line); + cl_git_exec( + git_status_should_ignore(&is_ignored, g_repo, filepath), 0, file, line); clar__assert( (expected != 0) == (is_ignored != 0), file, line, "expected != is_ignored", filepath, 1); diff --git a/tests/submodule/submodule_helpers.c b/tests/submodule/submodule_helpers.c index 6c2b9cf78..cd541ea86 100644 --- a/tests/submodule/submodule_helpers.c +++ b/tests/submodule/submodule_helpers.c @@ -204,7 +204,7 @@ void assert__submodule_exists( git_submodule *sm; int error = git_submodule_lookup(&sm, repo, name); if (error) - cl_git_report_failure(error, file, line, msg); + cl_git_report_failure(error, 0, file, line, msg); cl_assert_at_line(sm != NULL, file, line); git_submodule_free(sm); } From c52480fde52c6d65b8ded9ba65bce0b15ff312df Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Fri, 17 Feb 2017 13:01:49 +0000 Subject: [PATCH 3/3] `cl_git_exec` -> `cl_git_expect` --- tests/attr/ignore.c | 2 +- tests/clar_libgit2.h | 6 +++--- tests/repo/env.c | 12 ++++++------ tests/status/ignore.c | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/attr/ignore.c b/tests/attr/ignore.c index a8845e463..3a193890a 100644 --- a/tests/attr/ignore.c +++ b/tests/attr/ignore.c @@ -21,7 +21,7 @@ static void assert_is_ignored_( { int is_ignored = 0; - cl_git_exec( + cl_git_expect( git_ignore_path_is_ignored(&is_ignored, g_repo, filepath), 0, file, line); clar__assert_equal( diff --git a/tests/clar_libgit2.h b/tests/clar_libgit2.h index 1d77bc6d6..c72d37db3 100644 --- a/tests/clar_libgit2.h +++ b/tests/clar_libgit2.h @@ -12,11 +12,11 @@ * * Use this wrapper around all `git_` library calls that return error codes! */ -#define cl_git_pass(expr) cl_git_exec((expr), 0, __FILE__, __LINE__) +#define cl_git_pass(expr) cl_git_expect((expr), 0, __FILE__, __LINE__) -#define cl_git_fail_with(error, expr) cl_git_exec((expr), error, __FILE__, __LINE__) +#define cl_git_fail_with(error, expr) cl_git_expect((expr), error, __FILE__, __LINE__) -#define cl_git_exec(expr, expected, file, line) do { \ +#define cl_git_expect(expr, expected, file, line) do { \ int _lg2_error; \ giterr_clear(); \ if ((_lg2_error = (expr)) != expected) \ diff --git a/tests/repo/env.c b/tests/repo/env.c index 235f787ab..6404f88e9 100644 --- a/tests/repo/env.c +++ b/tests/repo/env.c @@ -56,8 +56,8 @@ static int GIT_FORMAT_PRINTF(2, 3) cl_setenv_printf(const char *name, const char static void env_pass_(const char *path, const char *file, int line) { git_repository *repo; - cl_git_exec(git_repository_open_ext(NULL, path, GIT_REPOSITORY_OPEN_FROM_ENV, NULL), 0, file, line); - cl_git_exec(git_repository_open_ext(&repo, path, GIT_REPOSITORY_OPEN_FROM_ENV, NULL), 0, file, line); + cl_git_expect(git_repository_open_ext(NULL, path, GIT_REPOSITORY_OPEN_FROM_ENV, NULL), 0, file, line); + cl_git_expect(git_repository_open_ext(&repo, path, GIT_REPOSITORY_OPEN_FROM_ENV, NULL), 0, file, line); cl_assert_at_line(git__suffixcmp(git_repository_path(repo), "attr/.git/") == 0, file, line); cl_assert_at_line(git__suffixcmp(git_repository_workdir(repo), "attr/") == 0, file, line); cl_assert_at_line(!git_repository_is_bare(repo), file, line); @@ -98,24 +98,24 @@ static void env_check_objects_(bool a, bool t, bool p, const char *file, int lin cl_git_pass(git_oid_fromstr(&oid_a, "45141a79a77842c59a63229403220a4e4be74e3d")); cl_git_pass(git_oid_fromstr(&oid_t, "1385f264afb75a56a5bec74243be9b367ba4ca08")); cl_git_pass(git_oid_fromstr(&oid_p, "0df1a5865c8abfc09f1f2182e6a31be550e99f07")); - cl_git_exec(git_repository_open_ext(&repo, "attr", GIT_REPOSITORY_OPEN_FROM_ENV, NULL), 0, file, line); + cl_git_expect(git_repository_open_ext(&repo, "attr", GIT_REPOSITORY_OPEN_FROM_ENV, NULL), 0, file, line); if (a) { - cl_git_exec(git_object_lookup(&object, repo, &oid_a, GIT_OBJ_BLOB), 0, file, line); + cl_git_expect(git_object_lookup(&object, repo, &oid_a, GIT_OBJ_BLOB), 0, file, line); git_object_free(object); } else { cl_git_fail_at_line(git_object_lookup(&object, repo, &oid_a, GIT_OBJ_BLOB), file, line); } if (t) { - cl_git_exec(git_object_lookup(&object, repo, &oid_t, GIT_OBJ_BLOB), 0, file, line); + cl_git_expect(git_object_lookup(&object, repo, &oid_t, GIT_OBJ_BLOB), 0, file, line); git_object_free(object); } else { cl_git_fail_at_line(git_object_lookup(&object, repo, &oid_t, GIT_OBJ_BLOB), file, line); } if (p) { - cl_git_exec(git_object_lookup(&object, repo, &oid_p, GIT_OBJ_COMMIT), 0, file, line); + cl_git_expect(git_object_lookup(&object, repo, &oid_p, GIT_OBJ_COMMIT), 0, file, line); git_object_free(object); } else { cl_git_fail_at_line(git_object_lookup(&object, repo, &oid_p, GIT_OBJ_COMMIT), file, line); diff --git a/tests/status/ignore.c b/tests/status/ignore.c index a9b96114b..251de39dd 100644 --- a/tests/status/ignore.c +++ b/tests/status/ignore.c @@ -20,7 +20,7 @@ static void assert_ignored_( bool expected, const char *filepath, const char *file, int line) { int is_ignored = 0; - cl_git_exec( + cl_git_expect( git_status_should_ignore(&is_ignored, g_repo, filepath), 0, file, line); clar__assert( (expected != 0) == (is_ignored != 0),