On Fri, Mar 25, 2016 at 2:06 AM, Pranit Bauva <pranit.bauva@xxxxxxxxx> wrote: > On Fri, Mar 25, 2016 at 5:27 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: >> Agreed that this needs proper justification in the commit message. >> And, the justification is to make each test more self-contained, >> particularly because a subsequent patch will introduce a second fake >> "editor", and by making tests responsible for setting the editor they >> need, they don't have to worry about bad interactions from "editors" >> set by earlier tests[1][2]. > > This shou cadve mbe ave be ave be ave be ave be ave be ave be ave Hmm, yes, what you say makes perfect sense... er, wait... >>>> -cat >check-for-diff <<EOF >>>> -#!$SHELL_PATH >>>> -exec grep '^diff --git' "\$1" >>>> +write_script "check-for-diff" <<-\EOF && >>>> +grep '^diff --git' "$1" >out && >>>> +test $(wc -l <out) = 1 >>> >>> Our test lib offers the test_line_count helper function, which >>> outputs a helpful error message in case the number of lines do not >>> match. >> >> test_line_count() was mentioned in [2], however, this line counting is >> done in the fake "editor" script, not in the test script proper, so >> the spelled-out form $(wc ...) was proposed[2]. > > I have a slight doubt regarding this. Can the functions from test-lib > work in such scripts flawlessly? If yes, then its probably better to > use them. Probably not easily, and it's not worth complicating the "editor" script by trying to import the test_line_count() function. >>>> chmod +x check-for-diff >> >> Drop the 'chmod' line; write_script() does this for you. > > I was unaware about this. I will drop it off. I thought I had mentioned it in the review in which I had suggested using write_script() or one of the follow-up emails, but upon looking back at those messages, I saw it was not so. It turns out that I was probably thinking about a different patch review in which I had mentioned dropping 'chmod'[1]. [1]: http://article.gmane.org/gmane.comp.version-control.git/288085/ -- 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