Convert unit test assertion helpers into macros #4009
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Reid Wahl <[email protected]>
Several unit tests are doing basically the same thing. This commit pulls out the parts that are common to all of them, using overrides for the ones that are unique to a particular test file. Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
This commit is much larger than I would prefer, but I didn't see a good way to do it in steps. Previously, a pcmk__output_factory_t created a pcmk__output_t object, NULL-checked it, and assigned all of the format-specific fields. This commit does the following: * Rename pcmk__output_factory_t to pcmk__output_setup_fn_t. * Accept an existing pcmk__output_t * argument in pcmk__output_setup_fn_t. * Return void from pcmk__output_setup_fn_t since it doesn't allocate the object. * Allocate the pcmk__output_t object in pcmk__bare_output_new() and pass it to the setup function. * Assign the request, register_message, and message fields within pcmk__bare_output_new(), since those are common to all setup functions. * Rename several fields and variables from "create" to "setup_fn". * Rename the pcmk__mk_FORMAT_output() functions to pcmk__output_setup_FORMAT(). * Rename the pcmk__output_null_create*() functions to pcmk__output_setup_dummy*(). I chose "setup" over "init" to avoid confusion with the pcmk__output_t:init method. Signed-off-by: Reid Wahl <[email protected]>
nrwahl2
force-pushed
the
nrwahl2-unit_test
branch
2 times, most recently
from
ddb29e7 to
57a2282
Compare
Signed-off-by: Reid Wahl <[email protected]>
nrwahl2
force-pushed
the
nrwahl2-unit_test
branch
from
57a2282 to
711ddd0
Compare
It could be static, except that it has unit tests. Signed-off-by: Reid Wahl <[email protected]>
Nothing uses it. pcmk__register_message() is always called directly. Most of the calls are in unit tests. The only other call is in pcmk__register_messages(). Considering that, it doesn't seem that we would gain much by calling it as a method of pcmk__output_t. Signed-off-by: Reid Wahl <[email protected]>
This is unfortunately the way to make a double-const (char **) (that is, the top-level pointer and the next-level pointers are const). It also unfortunately requires explicit casts. That was the initial motivation for all of the output setup shuffling in previous commits. I wanted fewer calls to pcmk__quote_cmdline(). Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
This is a step toward converting the assertion helper into macros. We'll need multiple macros to avoid complaints from the compiler or Coverity about NULL arguments, always-true/always-false conditions, etc. Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
This is a step toward converting the assertion helpers into macros. We'll need multiple macros to avoid complaints from the compiler or Coverity about NULL arguments, always-true/always-false conditions, etc. In this case we could probably use variadic arguments, but the error messages on failed assertions would be less clear. At best, we could show that the result is not equal to the last expected string (the alternate). Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
I don't see how these were useful. Also use all four combinations of uid/gid NULL/non-NULL for each test. Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Do this unconditionally. We don't need to be terribly concerned with efficiency here. This will make the upcoming conversion to a macro simpler. Signed-off-by: Reid Wahl <[email protected]>
...to assert_compare_versions(). Signed-off-by: Reid Wahl <[email protected]>
With a function, the CMocka assert macros showed unhelpful line numbers on error. We want to convert these to macros. These splits have two goals: * Make each helper a bit easier to read. * Avoid complaints from the compiler or Coverity. I started making splits after compilation failed due to a NULL strcmp() argument, even though strcmp() would have never actually been *called* with a NULL argument. Macro expansion has unpleasant results sometimes. Signed-off-by: Reid Wahl <[email protected]>
This is a step toward replacing assert_comment() with a macro completely. With a function, the CMocka assert macros show unhelpful line numbers on error. We'll need to split up assert_comment() into a NULL case and a non-NULL case. The code that gets split out in this commit will be shared between them. Signed-off-by: Reid Wahl <[email protected]>
This is a step toward converting the assertion helpers into macros. Splitting into multiple helpers has two goals. * Make each helper a bit easier to read. * Avoid complaints from the compiler or Coverity. I started making splits after compilation failed due to a NULL strcmp() argument, even though strcmp() would have never actually been *called* with a NULL argument. Macro expansion has unpleasant results sometimes. In the case of the pcmk__xc_create() tests, there's no need to even keep wrapper functions for the NULL and non-NULL cases, since each would be used only once. Signed-off-by: Reid Wahl <[email protected]>
With a function, the CMocka assert macros showed unhelpful line numbers on error. We want to convert these to macros. Splitting into multiple helpers has two goals. * Make each helper a bit easier to read. * Avoid complaints from the compiler or Coverity. I started making splits after compilation failed due to a NULL strcmp() argument, even though strcmp() would have never actually been *called* with a NULL argument. Macro expansion has unpleasant results sometimes. Signed-off-by: Reid Wahl <[email protected]>
...in pcmk__is_name_char() and pcmk__is_name_start_char() unit tests. We need to treat these specially (see comments). The end goal is to convert the helper functions to macros. With a function, the CMocka assert macros show unhelpful line numbers on error. I'm doing this with two goals: * Make the helpers a bit easier to read. * Avoid complaints from the compiler or Coverity. I started making splits after compilation failed due to a NULL strcmp() argument, even though strcmp() would have never actually been *called* with a NULL argument. Macro expansion has unpleasant results sometimes. I'm paranoid that more things like this might appear. Signed-off-by: Reid Wahl <[email protected]>
And drop redundant extern declarations. They're already part of the included headers. Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
By the time we were freeing list, it was already NULL. Signed-off-by: Reid Wahl <[email protected]>
This will make some planned changes easier. Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
...by ensuring that reference is non-NULL in all callers. Signed-off-by: Reid Wahl <[email protected]>
With a function, the CMocka assert macros showed unhelpful line numbers on error. Ref T222 Signed-off-by: Reid Wahl <[email protected]>
All current callers pass non-NULL device. Signed-off-by: Reid Wahl <[email protected]>
This is to match the signature of g_strv_contains(). Unfortunately, it requires casts at each caller. However, since the plan is to replace this function with g_strv_contains(), we might as well do this now. Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
nrwahl2
force-pushed
the
nrwahl2-unit_test
branch
3 times, most recently
from
e2d504a to
4055fda
Compare
Signed-off-by: Reid Wahl <[email protected]>
In the remaining places I could find where it hasn't already been done. This is annoying to look at, but it's correct. char ** and gchar ** are obviously not const, and we should use const where we can. The frustrating thing is that this pointer type is not compatible with char **, so it requires explicit casts. Signed-off-by: Reid Wahl <[email protected]>
nrwahl2
force-pushed
the
nrwahl2-unit_test
branch
from
4055fda to
d6a2ffc
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See #3970 (comment). This turned into a pretty big pain and time sink. I'm glad it's done.