On Wed, Aug 17, 2016 at 12:09:25PM -0700, Junio C Hamano wrote:
> Brian Henderson <henderson.bj@xxxxxxxxx> writes:
<snip>
>
> > +
> > +# PERL is required, but assumed to be present, although not necessarily modern
> > +# some tests require 5.8
> > +test_expect_success PERL 'name' 'true'
>
> If the platform lacks PERL prerequisite, this will simply be
> skipped, and if the platform has it, it will always succeed.
>
> I am not sure what you are trying to achieve by having this line
> here.
I originally didn't have this line, and my comment was referring to the
t/README which says
Even without the PERL prerequisite, tests can assume there is a
usable perl interpreter at $PERL_PATH, though it need not be
particularly modern.
There is current functionality in diff-highlight which requires at least
perl 5.8 (the utf8 functions). I was going to add a test for this as
well, but I'm not super comfy with multibyte chars.
Eric recommended adding this line, what do you think?
would `test_set_prereq PERL` be better?
>
> > +test_expect_success 'diff-highlight does not highlight whole line' '
> > + dh_test \
> > + "aaa\nbbb\nccc\n" \
> > + "aaa\n000\nccc\n"
> > +'
This (at least to me) is desired. See comment for `sub
is_pair_interesting`
>
> Hmm, does this express the desired outcome, or just document the
> current (possibly broken--I dunno) behaviour? The same question for
> the next one.
>
> > +test_expect_success 'diff-highlight does not highlight mismatched hunk size' '
> > + dh_test \
> > + "aaa\nbbb\n" \
> > + "aaa\nb0b\nccc\n"
> > +'
This is undesired behavior, but currently implemented for simplicity,
see `sub show_hunk`
Do they need comments or something?
<snip>
>
> > + test -s diff.act &&
>
> Why? If you always have the expected output that you are going to
> compare with, wouldn't that sufficient to do that test without this?
> Besides, having "test -s" means that you can never make sure that a
> certain pair of input does not show any changes. Perhaps drop it?
I was trying to address Eric's concern for `printf` or `git commit` et
al failing. Also, this file will always be a diff, it just might not
having any highlighting (so not empty?).
I'll take another stab.
>
> > + diff diff.exp diff.act
>
> Use test_cmp unless there is a strong reason why you shouldn't?
>
> > +}
> > +
> > +dh_commit_test() {
> > + a="$1" b="$2"
> > +
> > + printf "$a" >file
> > + git add file
> > + git commit -m"Add a file" >/dev/null
>
> Avoid sticking a short-option to its argument, i.e.
>
> git commit -m "Add a file"
>
> > +
> > + printf "$b" >file
> > + git commit -am"Update a file" >/dev/null
>
> Likewise.
>
> git commit -a -m "Update a file"
>
> The remainder of the file invites the same set of questions and
> comments you see for dh_diff_test() above, so I won't repeat them.
>
> Thanks.
thanks for the feedback.
--
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