Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Improve clar messages#4130

Merged
ethomson merged 3 commits intomasterlibgit2/libgit2:masterfrom
ethomson/clar_messageslibgit2/libgit2:ethomson/clar_messagesCopy head branch name to clipboard
Feb 17, 2017
Merged

Improve clar messages#4130
ethomson merged 3 commits intomasterlibgit2/libgit2:masterfrom
ethomson/clar_messageslibgit2/libgit2:ethomson/clar_messagesCopy head branch name to clipboard

Conversation

@ethomson
Copy link
Member

@ethomson ethomson commented Feb 17, 2017

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.

cl_git_pass failures look unchanged:

  1) Failure:
apply::fromfile::test_name [tests/apply/fromfile.c:42]
  Function call failed: (git_patch_from_buffer(&patch, "", 0, ((void *)0)))
  error -3 - no patch found

cl_git_fail messages now:

  1) Failure:
apply::fromfile::test_name [tests/apply/fromfile.c:42]
  Function call succeeded: git_patch_from_buffer(&patch, patchfile, strlen(patchfile), NULL)
  no error, expected non-zero return

cl_git_fail_with messages now:

  1) Failure:
apply::fromfile::test_name [tests/apply/fromfile.c:42]
  Function call failed: (git_patch_from_buffer(&patch, "", 0, ((void *)0)))
  error -3 (expected -99) - no patch found

/cc @pks-t

p_snprintf: no need for arguments to a format
cc17264 
`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__)
```
@ethomson
Copy link
Member Author

ethomson commented Feb 17, 2017

The goal here is to see more information about why some tests fail in appveyor. We expect certificate failures, but are getting a generic -1 failure. Shoddy networking, but it would be interesting to see a bit more details.

@pks-t
Copy link
Member

pks-t commented Feb 17, 2017

Very nice.

You forgot to modfiy all existing callsites of cl_git_report_failure ;)

What do you think about using cl_git_expect instead of cl_git_exec as the macro's name?

Edward Thomson added 2 commits February 17, 2017 12:58
tests: provide better pass/failure error messages
a1dcc83 
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.
cl_git_exec -> cl_git_expect
c52480f
@ethomson ethomson force-pushed the ethomson/clar_messages branch from fb2ef7b to c52480f Compare February 17, 2017 13:02
@ethomson
Copy link
Member Author

ethomson commented Feb 17, 2017

I did forget win32! Oops!

I had this as cl_git_expect or something similar to begin with, so I was rather on the fence. It sounds like you prefer that, so I've changed cl_git_exec -> cl_git_expect.

@pks-t
Copy link
Member

pks-t commented Feb 17, 2017

cl_git_exec did not really sound as if it would check anything but only execute a function. So I think cl_git_expect makes the function a bit more obvious in its behavior.

Thanks, looks good to me.

@ethomson ethomson merged commit b13f0da into master Feb 17, 2017
@johnhaley81 johnhaley81 mentioned this pull request Apr 20, 2017
Merged
@ethomson ethomson deleted the ethomson/clar_messages branch January 9, 2019 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ethomson @pks-t
Morty Proxy This is a proxified and sanitized view of the page, visit original site.