Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Very minor changes to some tests #254

Merged
merged 1 commit into from Aug 13, 2016
Merged

Very minor changes to some tests #254

merged 1 commit into from Aug 13, 2016

Conversation

ghost
Copy link

@ghost ghost commented Aug 8, 2016

  • C89 comment > C99 comment
  • Less long lines (for small screens and see 2 files as the same with a "multiple panels" feature like the one in GNU Emacs and Qt Creator)
  • return STDC_CONST for main
  • Errors to stderr
  • Other things

@hawicz
Copy link
Member

hawicz commented Aug 9, 2016

This looks generally ok, but there are a couple of changes in there that look like they'll affect the output of the tests. i.e. output to stderr in test4.c, and elimination of a newline in test_null.c.
Why are those needed?

@ghost
Copy link
Author

ghost commented Aug 9, 2016

How could be a problem to print errors to stderr instead of stdout? It is the current behavior that seems strange to me.

puts had a new line. http://www.cplusplus.com/reference/cstdio/puts/
I fixed the place where I did the confusion with fputs.

@hawicz
Copy link
Member

hawicz commented Aug 10, 2016

Re: puts, ok.

As for stdout vs stderr, the tests write their output to a file and then that file is compared with the expected output. Currently, only stdout is redirected, so if you start emitting messages to stderr it will no longer be possible to see what happened by simply looking at the output file.
That could be fixed by redirecting both stdout and stderr, but then what's the point of changing the output to go to stderr in the first place?

@ghost
Copy link
Author

ghost commented Aug 12, 2016

I amended the commit in order not to change the stream of the prints.

@hawicz hawicz merged commit 8215c0a into json-c:master Aug 13, 2016
@ghost ghost deleted the tests branch August 13, 2016 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant