Tanay Abhra <tanayabh@xxxxxxxxx> writes: > +test_expect_success 'clear default config' ' > + rm -f .git/config > +' > + > +cat > .git/config << EOF t/README says: - Put all code inside test_expect_success and other assertions. Even code that isn't a test per se, but merely some setup code should be inside a test assertion. Even these cat > would better be in a test_expect_success 'initialize config'. (Not applied everywhere in Git's code essentially because some tests were written before the guideline was set and never updated). > +[core] > + penguin = very blue > + Movie = BadPhysics > + UPPERCASE = true > + MixedCase = true > + my = > + foo > + baz = sam > +[Cores] > + WhatEver = Second > +[my "Foo bAr"] > + hi = hello To really stress the "case sensitive middle part" case, you should also have other sections like [my "foo bar"] hi = lower-case [my "FOO BAR"] hi = upper-case and check that you get the right value for my.*.hi Similarly, I'd add a [CORE] and a [CoRe] section to check that their content is actually merged with [core]. > +test_expect_success 'get value for a key with value as an empty string' ' > + echo "" >expect && > + test-config get_value core.my >actual && > + test_cmp expect actual > +' > + > +test_expect_success 'get value for a key with value as NULL' ' > + echo "(NULL)" >expect && > + test-config get_value core.foo >actual && > + test_cmp expect actual > +' > +test_expect_success 'upper case key' ' Keep the style consistent, if you separate tests with a single blank line, do it everywhere. > +cat > expect << EOF See above, should be in test_expect_success. Also, >expect, not > expect. There are other instances. > +1 > +0 > +1 > +1 > +1 > +EOF > + > +test_expect_success 'find bool value for the entered key' ' > + test-config get_bool goat.head >>actual && The first one should be a single >, or you should clear actual before the test. > +int main(int argc, char **argv) > +{ > + int i, no_of_files; > + int ret = 0; > + const char *v; > + int val; > + const struct string_list *strptr; > + struct config_set cs = CONFIG_SET_INIT; > + if (argc == 3 && !strcmp(argv[1], "get_value")) { > + if (!git_config_get_value(argv[2], &v)) { > + if (!v) > + printf("(NULL)\n"); > + else > + printf("%s\n", v); > + return 0; > + } else { > + printf("Value not found for \"%s\"\n", argv[2]); > + return -1; > + } > + } else if (argc == 3 && !strcmp(argv[1], "get_value_multi")) { > + strptr = git_config_get_value_multi(argv[2]); > + if (strptr) { > + for (i = 0; i < strptr->nr; i++) { > + v = strptr->items[i].string; > + if (!v) > + printf("(NULL)\n"); > + else > + printf("%s\n", v); > + } > + return 0; > + } else { > + printf("Value not found for \"%s\"\n", argv[2]); > + return -1; > + } > + } else if (argc == 3 && !strcmp(argv[1], "get_int")) { > + if (!git_config_get_int(argv[2], &val)) { > + printf("%d\n", val); > + return 0; > + } else { > + printf("Value not found for \"%s\"\n", argv[2]); > + return -1; > + } > + } else if (argc == 3 && !strcmp(argv[1], "get_bool")) { > + if (!git_config_get_bool(argv[2], &val)) { > + printf("%d\n", val); > + return 0; > + } else { > + printf("Value not found for \"%s\"\n", argv[2]); > + return -1; > + } > + } else if (!strcmp(argv[1], "configset_get_value")) { > + no_of_files = git_config_int("unused", argv[2]); Why ask the user to give a number of files on the command-line. With a syntax like test-config configset_get_value <key> <files>... you could just use argc to iterate over argv. Here, you trust the user to provide the right value, and most likely segfault otherwise (and this is not really documented). I know this is only test code, but why not do it right anyway ;-). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html