Re: [PATCH v3 02/10] t5526: stop asserting on stderr literally
- From
Glen Choo <chooglen@google.com>
- Date
- Feb 24, 2022, 16:15 UTC
- Message-ID
- <kl6lpmnc44wu.fsf@chooglen-macbookpro.roam.corp.google.com>
- In-Reply-To
- <220224.8635k8a36n.gmgdl@evledraar.gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 15 quoted lines
> On Thu, Feb 24 2022, Glen Choo wrote:
>
>> +check_sub() {
>> + NEW_HEAD=$1 &&
>> + cat <<-EOF >$pwd/expect.err.sub
>
> Hrm, I didn't know that would work, the usual style is:
>
> cat >file <<...
>
> Instead of:
>
> cat <<.. >file
>
> Maybe better to use that?Thanks, I somehow mixed things up when I wrote that.
Show 29 quoted lines
>> + Fetching submodule submodule
>> + From $pwd/submodule
>> + OLD_HEAD..$NEW_HEAD sub -> origin/sub
>> + EOF
>> +}
>> +
>> +check_deep() {
>> + NEW_HEAD=$1 &&
>> + cat <<-EOF >$pwd/expect.err.deep
>> + Fetching submodule submodule/subdir/deepsubmodule
>> + From $pwd/deepsubmodule
>> + OLD_HEAD..$NEW_HEAD deep -> origin/deep
>> + EOF
>> +}
>> +
>> +check_super() {
>> + NEW_HEAD=$1 &&
>> + cat <<-EOF >$pwd/expect.err.super
>> + From $pwd/.
>> + OLD_HEAD..$NEW_HEAD super -> origin/super
>> + EOF
>> +}
>
> These look a lot better, but instead of always passing the result of
> "git rev-parse --short HEAD" can't we just always invoke that in these
> helpers?
>
> Maybe there are cases where $NEW_HEAD is different, I've just skimmed
> this series.I haven't found any other instances where $NEW_HEAD is different, so I suppose we could move it into the helpers. I don't think it benefits readability that much to do so, but if you think it's much better, I'll incorporate it when I reroll this.
Show 12 quoted lines
>> @@ -62,7 +82,8 @@ verify_fetch_result() {
>> if [ -f expect.err.deep ]; then
>> cat expect.err.deep >>expect.err.combined
>> fi &&
>> - test_cmp expect.err.combined $ACTUAL_ERR
>> + sed -E 's/[0-9a-f]+\.\./OLD_HEAD\.\./' $ACTUAL_ERR >actual.err.cmp &&
>> + test_cmp expect.err.combined actual.err.cmp
>> }
>
> I think this is unportable per check-non-portable-shell.pl:
>
> /\bsed\s+-[^efn]\s+/ and err 'sed option not portable (use only -n, -e, -f)';Ah thanks, my sed-fu is pretty poor, so I appreciate the tip :)
I used that because I wanted +, but I found what I needed from the sed manpage i.e. that + is equivalent to \{1,\}).