On Fri, Mar 25, 2016 at 11:54 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > 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... haha :) Sorry for this. My browser crashed and it sent out some crap (don't know how). I mean to say, "This should have been mentioned in the commit message that scope of editor is reduced so as to not worry about bad interactions from other "editors" which may be used after wards." > >>>>> -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. Sure! >>>>> 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/ Oh! I hadn't tested by removing chmod. I will try it for fun. -- 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