On 02.08.2012 23:00, Junio C Hamano wrote:
Adam Butcher <dev.lists@xxxxxxxxxxxxxxx> writes:
+# create a file containing numbers with no newline at
+# the end and modify it such that the starting 10 lines
+# are unchanged, the next 101 are rewritten and the last
+# line differs only in that in is terminated by a newline.
+seq 1 10 > seq
+seq 100 +1 200 >> seq
+printf 201 >> seq
+(git add seq; git commit seq -m seq) >/dev/null
+seq 1 10 > seq
+seq 300 -1 200 >> seq
We would prefer to have these set-up steps in test_expect_success.
That way, we will have more chance to catch potential and unintended
breakage to "git add" and "git commit" when people attempt to update
them.
Cool, no probs. I had originally put them at the start of the first
test that I added but decided to pull them out as prep. I think that
msysGit or something about my Windows shell session may have played a
part in my not chaining them with && also (see below). I'll clean them
up and wrap them in a test_expect_success.
Also, the redirect target sticks to redirect operator in our
scripts, i.e. "cmd >seq" not "cmd > seq".
Okay, will change.
+test_expect_success 'no newline at eof is on its own line without
-B'
+
+ (git diff seq; true) > res &&
What is this subshell and true about? A git diff does not exit with
non zero to signal differences,
Hmm, (?confused?) yeah actually I didn't think it did -- I was
surprised when git returned 1 for this line. I think it must have been
an issue with the version msysGit I was using or something sticking
errorlevel in my Windows shell. Git seemed to return 1 ALWAYS! I
usually use gnu/linux but on this occasion I wrote the fix and tests
blind on a Windows machine testing the logic manually with msysGit. I
ran the tests on a linux machine at work and they did what I expected so
I left them as was without rechecking this.
I'm glad that this can be simplified. It felt wrong -- similar lines
elsewhere in the script didn't do it so I wasn't really happy with it.
Turns out it looks to be a Windows/environment issue. I cannot
reproduce it now.
and even if it did, the right way to
write it would be
test_might_fail git cmd >res &&
Fair enough. Good to know that's available.
to allow us to make sure that the git command that may or may not
exit with zero still does not die an uncontrolled death (e.g. segv).
+ grep "^\\\\ No newline at end of file$" res &&
+ grep -v "^.\\+\\\\ No newline at end of file" res &&
+ grep -v "\\\\ No newline at end of file.\\+$" res
+'
It is preferrable not to spell "No newline at ..." part out, so that
we won't have to worry about future rewords and i18n.
Okay no probs. I was originally going to spell it out only once and
use parameter expansion. However I understand the point of not spelling
it out at all. The only reason I did so was to catch other potential
errors where text may have been 'attached' to either side of the
annotation string by some future (or other) bug.
There are older
tests that predate i18n and they do spell these out, but that is not
a good reason to make things worse than they already are.
Agreed. Should I just test for this prefix case (i.e. the bug at hand)
only and not preempt future potential issues? Or should I just stick
the current string in a variable and keep the logic as is; at least then
there would only be one place requiring a fix in the reword case (but
still additional rework in the i18n case -- though I assume the test
could force a particular locale to evade this).
"git apply" only looks at the backslash-space at the beginning of
line anyway.
Okay.
+test_expect_success 'no newline at eof is on its own line with -B'
'
+
+ (git diff -B seq; true) > res &&
+ grep "^\\\\ No newline at end of file$" res &&
+ grep -v "^.\\+\\\\ No newline at end of file" res &&
+ grep -v "\\\\ No newline at end of file.\\+$" res
+'
Likewise.
test_done
Thanks.
No probs. I will address both your and Jeff's comments sometime
tomorrow and hopefully send a well formatted patch in next time.
Cheers,
Adam
--
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