Patchwork [3,of,3] tests: extend check-commit self-tests

login
register
mail settings
Submitter Augie Fackler
Date Jan. 11, 2016, 1:29 a.m.
Message ID <9D133EE5-91B3-4896-8B6D-F08B26F4196B@durin42.com>
Download mbox | patch
Permalink /patch/12651/
State Accepted
Commit 374fad80ce6975447d7abf55c5127e776f1ad4b6
Headers show

Comments

Augie Fackler - Jan. 11, 2016, 1:29 a.m.
> On Jan 10, 2016, at 8:13 PM, Matt Mackall <mpm@selenic.com> wrote:
> 
> On Sat, 2016-01-09 at 00:21 -0500, Augie Fackler wrote:
>>> On Jan 8, 2016, at 1:46 PM, Matt Mackall <mpm@selenic.com> wrote:
>>> 
>>> # HG changeset patch
>>> # User Matt Mackall <mpm@selenic.com>
>>> # Date 1452276435 21600
>>> #      Fri Jan 08 12:07:15 2016 -0600
>>> # Node ID a8ea9d699407fcbdb44c87a097f32432039ca483
>>> # Parent  54875580a56db3682f5dc4cadd65c7b6d9176cd7
>>> tests: extend check-commit self-tests
>> 
>> 1 and 2 are clowncopterized, but 3 fails tests for me. Can you take a look and
>> resend?
> 
> Still works for me after rebasing, you'll need to be more specific.



> Is your patch flow swallowing trailing spaces?

I sincerely doubt that - many things would have been broken for years were that the case. Generally patches won’t even apply if there’s trailing whitespace damage.

> 
> --
> Mathematics is the supreme nostalgia of our time.
>
Matt Mackall - Jan. 11, 2016, 5:45 p.m.
On Sun, 2016-01-10 at 20:29 -0500, Augie Fackler wrote:
> > On Jan 10, 2016, at 8:13 PM, Matt Mackall <mpm@selenic.com> wrote:
> > 
> > On Sat, 2016-01-09 at 00:21 -0500, Augie Fackler wrote:
> > > > On Jan 8, 2016, at 1:46 PM, Matt Mackall <mpm@selenic.com> wrote:
> > > > 
> > > > # HG changeset patch
> > > > # User Matt Mackall <mpm@selenic.com>
> > > > # Date 1452276435 21600
> > > > #      Fri Jan 08 12:07:15 2016 -0600
> > > > # Node ID a8ea9d699407fcbdb44c87a097f32432039ca483
> > > > # Parent  54875580a56db3682f5dc4cadd65c7b6d9176cd7
> > > > tests: extend check-commit self-tests
> > > 
> > > 1 and 2 are clowncopterized, but 3 fails tests for me. Can you take a look
> > > and
> > > resend?
> > 
> > Still works for me after rebasing, you'll need to be more specific.
> 
> --- /home/augie/hg/tests/test-check-commit.t
> +++ /home/augie/hg/tests/test-check-commit.t.err
> @@ -20,5 +20,14 @@
>    >        echo
>    >   fi
>    > done
> +  Revision a7324f739d78 does not comply to rules
> +  ------------------------------------------------------
> +  57: (BC) needs to be uppercase
> +   +  > transplant/foo: this summary is way too long use Oxford comma (bc)
> (bug123) (issue 244)
> +  57: no space allowed between issue and number
> +   +  > transplant/foo: this summary is way too long use Oxford comma (bc)
> (bug123) (issue 244)
> +  57: use (issueDDDD) instead of bug
> +   +  > transplant/foo: this summary is way too long use Oxford comma (bc)
> (bug123) (issue 244)

Ahh. These are all false positives: it's running check-commit on your draft
commits and finding intentionally bad jargon in the body of a commit that's
testing detection of said jargon. They weren't triggering for me because my
local csets were secret.

Short of introducing a no-check-commit mechanism or a way of putting escaping in
run-test commands, there's no simple fix. On the other hand, it'll cease to be
an issue as soon as the change is marked public.

-- 
Mathematics is the supreme nostalgia of our time.
Augie Fackler - Jan. 11, 2016, 5:48 p.m.
> On Jan 11, 2016, at 12:45, Matt Mackall <mpm@selenic.com> wrote:
> 
> On Sun, 2016-01-10 at 20:29 -0500, Augie Fackler wrote:
>>> On Jan 10, 2016, at 8:13 PM, Matt Mackall <mpm@selenic.com> wrote:
>>> 
>>> On Sat, 2016-01-09 at 00:21 -0500, Augie Fackler wrote:
>>>>> On Jan 8, 2016, at 1:46 PM, Matt Mackall <mpm@selenic.com> wrote:
>>>>> 
>>>>> # HG changeset patch
>>>>> # User Matt Mackall <mpm@selenic.com>
>>>>> # Date 1452276435 21600
>>>>> #      Fri Jan 08 12:07:15 2016 -0600
>>>>> # Node ID a8ea9d699407fcbdb44c87a097f32432039ca483
>>>>> # Parent  54875580a56db3682f5dc4cadd65c7b6d9176cd7
>>>>> tests: extend check-commit self-tests
>>>> 
>>>> 1 and 2 are clowncopterized, but 3 fails tests for me. Can you take a look
>>>> and
>>>> resend?
>>> 
>>> Still works for me after rebasing, you'll need to be more specific.
>> 
>> --- /home/augie/hg/tests/test-check-commit.t
>> +++ /home/augie/hg/tests/test-check-commit.t.err
>> @@ -20,5 +20,14 @@
>>    >        echo
>>    >   fi
>>    > done
>> +  Revision a7324f739d78 does not comply to rules
>> +  ------------------------------------------------------
>> +  57: (BC) needs to be uppercase
>> +   +  > transplant/foo: this summary is way too long use Oxford comma (bc)
>> (bug123) (issue 244)
>> +  57: no space allowed between issue and number
>> +   +  > transplant/foo: this summary is way too long use Oxford comma (bc)
>> (bug123) (issue 244)
>> +  57: use (issueDDDD) instead of bug
>> +   +  > transplant/foo: this summary is way too long use Oxford comma (bc)
>> (bug123) (issue 244)
> 
> Ahh. These are all false positives: it's running check-commit on your draft
> commits and finding intentionally bad jargon in the body of a commit that's
> testing detection of said jargon. They weren't triggering for me because my
> local csets were secret.

Ah. In that case, why don't you just push this patch after you've pulled and reviewed the clowncopter so we don't have to stare at these for a long time.

Thanks!

> 
> Short of introducing a no-check-commit mechanism or a way of putting escaping in
> run-test commands, there's no simple fix. On the other hand, it'll cease to be
> an issue as soon as the change is marked public.
> 
> -- 
> Mathematics is the supreme nostalgia of our time.
> 
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
timeless - Jan. 11, 2016, 5:49 p.m.
Wouldn't my fixes help here?
They scope the check-commit to look for summaries as summaries and not
w/in things...

On Mon, Jan 11, 2016 at 12:45 PM, Matt Mackall <mpm@selenic.com> wrote:
> On Sun, 2016-01-10 at 20:29 -0500, Augie Fackler wrote:
>> > On Jan 10, 2016, at 8:13 PM, Matt Mackall <mpm@selenic.com> wrote:
>> >
>> > On Sat, 2016-01-09 at 00:21 -0500, Augie Fackler wrote:
>> > > > On Jan 8, 2016, at 1:46 PM, Matt Mackall <mpm@selenic.com> wrote:
>> > > >
>> > > > # HG changeset patch
>> > > > # User Matt Mackall <mpm@selenic.com>
>> > > > # Date 1452276435 21600
>> > > > #      Fri Jan 08 12:07:15 2016 -0600
>> > > > # Node ID a8ea9d699407fcbdb44c87a097f32432039ca483
>> > > > # Parent  54875580a56db3682f5dc4cadd65c7b6d9176cd7
>> > > > tests: extend check-commit self-tests
>> > >
>> > > 1 and 2 are clowncopterized, but 3 fails tests for me. Can you take a look
>> > > and
>> > > resend?
>> >
>> > Still works for me after rebasing, you'll need to be more specific.
>>
>> --- /home/augie/hg/tests/test-check-commit.t
>> +++ /home/augie/hg/tests/test-check-commit.t.err
>> @@ -20,5 +20,14 @@
>>    >        echo
>>    >   fi
>>    > done
>> +  Revision a7324f739d78 does not comply to rules
>> +  ------------------------------------------------------
>> +  57: (BC) needs to be uppercase
>> +   +  > transplant/foo: this summary is way too long use Oxford comma (bc)
>> (bug123) (issue 244)
>> +  57: no space allowed between issue and number
>> +   +  > transplant/foo: this summary is way too long use Oxford comma (bc)
>> (bug123) (issue 244)
>> +  57: use (issueDDDD) instead of bug
>> +   +  > transplant/foo: this summary is way too long use Oxford comma (bc)
>> (bug123) (issue 244)
>
> Ahh. These are all false positives: it's running check-commit on your draft
> commits and finding intentionally bad jargon in the body of a commit that's
> testing detection of said jargon. They weren't triggering for me because my
> local csets were secret.
>
> Short of introducing a no-check-commit mechanism or a way of putting escaping in
> run-test commands, there's no simple fix. On the other hand, it'll cease to be
> an issue as soon as the change is marked public.
>
> --
> Mathematics is the supreme nostalgia of our time.
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Matt Mackall - Jan. 11, 2016, 7:18 p.m.
On Mon, 2016-01-11 at 12:49 -0500, timeless wrote:
> Wouldn't my fixes help here?
> They scope the check-commit to look for summaries as summaries and not
> w/in things...

Probably. But it's also arguable that "(issue XXXX)" -anywhere- (descriptions,
comments, etc.) is bad style. With the exception of this test.

-- 
Mathematics is the supreme nostalgia of our time.

Patch

--- /home/augie/hg/tests/test-check-commit.t
+++ /home/augie/hg/tests/test-check-commit.t.err
@@ -20,5 +20,14 @@ 
   >        echo
   >   fi
   > done
+  Revision a7324f739d78 does not comply to rules
+  ------------------------------------------------------
+  57: (BC) needs to be uppercase
+   +  > transplant/foo: this summary is way too long use Oxford comma (bc) (bug123) (issue 244)
+  57: no space allowed between issue and number
+   +  > transplant/foo: this summary is way too long use Oxford comma (bc) (bug123) (issue 244)
+  57: use (issueDDDD) instead of bug
+   +  > transplant/foo: this summary is way too long use Oxford comma (bc) (bug123) (issue 244)
+

That’s on the GCC compile farm.