RFC: teuthology field in commit messages

RFC: teuthology field in commit messages

[Loic Dachary]

[-- Attachment #1: Type: text/plain, Size: 398 bytes --]

Hi Ceph,

An optional teuthology field could be added to a commit message like so:

teuthology: --suite rbd

to state that this commit should be tested with the rbd suite. It could be parsed by bots and humans.

It would make it easy and cost effective to run partial teuthology suites automatically on pull requests.

What do you think ?

-- 
Loïc Dachary, Artisan Logiciel Libre


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: RFC: teuthology field in commit messages

[Joao Eduardo Luis]

On 11/28/2015 03:56 PM, Loic Dachary wrote:
> Hi Ceph,
> 
> An optional teuthology field could be added to a commit message like so:
> 
> teuthology: --suite rbd
> 
> to state that this commit should be tested with the rbd suite. It could be parsed by bots and humans.
> 
> It would make it easy and cost effective to run partial teuthology suites automatically on pull requests.
> 
> What do you think ?

Can't we use git-notes for that instead?

I think this pollutes the history a bit. Especially considering this
sort of metadata isn't necessarily specific to a given diff.

Also should be considered that this is a field that may make sense today
but may not make much sense in 10, 15 years. And while we have quite a
few special-purpose fields (e.g., Fixes, Backport), those are currently
pretty explanatory and I believe will be still easily understandable in
a decade's time.

In any case, if there's absolutely no other way to do this and the other
folk thinks it's important to have this, I will certainly not be the
party pooper ;)

  -Joao

Re: RFC: teuthology field in commit messages

[Loic Dachary]

[-- Attachment #1: Type: text/plain, Size: 2369 bytes --]

Hi Joao,

On 29/11/2015 12:51, Joao Eduardo Luis wrote:
> On 11/28/2015 03:56 PM, Loic Dachary wrote:
>> Hi Ceph,
>>
>> An optional teuthology field could be added to a commit message like so:
>>
>> teuthology: --suite rbd
>>
>> to state that this commit should be tested with the rbd suite. It could be parsed by bots and humans.
>>
>> It would make it easy and cost effective to run partial teuthology suites automatically on pull requests.
>>
>> What do you think ?
> 
> Can't we use git-notes for that instead?

It possible but few people understand how it works.

> I think this pollutes the history a bit. Especially considering this
> sort of metadata isn't necessarily specific to a given diff.

I think it is relevant in a permanent way. When running a suite, we do it on a given diff. For instance,
in a 10 commit pull request, we run the suite on the head of the branch, which will later become the second parent of the merge. Should we want to test at a later time, long after the pull request has been merged, we will be able to do it using the same suite. 

> Also should be considered that this is a field that may make sense today
> but may not make much sense in 10, 15 years. And while we have quite a
> few special-purpose fields (e.g., Fixes, Backport), those are currently
> pretty explanatory and I believe will be still easily understandable in
> a decade's time.

It also holds for stable branches since we maintain stable branches for ceph-qa-suite as well. So, for backporting 3 commits from a given pull request, it will also help to know that the backport could also be tested with this specific suite. And if the suite is missing the test, it's also a good hint that this test needs to be backported as well.

> In any case, if there's absolutely no other way to do this and the other
> folk thinks it's important to have this, I will certainly not be the
> party pooper ;)

:-) FWIW, I think the Backport: field should not be used ( see http://tracker.ceph.com/projects/ceph-releases/wiki/HOWTO_schedule_an_issue_for_backporting#Backport-field-in-the-commit-messages for the full rationale ). But I think the "teuthology" field being used *prior* to the pull request being merged makes sense and is a valuable addition to the commit history.

Cheers

-- 
Loïc Dachary, Artisan Logiciel Libre


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: RFC: teuthology field in commit messages

[John Spray]

On Sat, Nov 28, 2015 at 3:56 PM, Loic Dachary <loic@dachary.org> wrote:
> Hi Ceph,
>
> An optional teuthology field could be added to a commit message like so:
>
> teuthology: --suite rbd
>
> to state that this commit should be tested with the rbd suite. It could be parsed by bots and humans.
>
> It would make it easy and cost effective to run partial teuthology suites automatically on pull requests.
>
> What do you think ?

Hmm, we are usually testing things at the branch/PR level rather than
on the per-commit level, so it feels a bit strange to have this in the
commit message.

However, if a system existed that would auto-test things when I put
something magic in a commit message, I would probably use it!

John


>
> --
> Loïc Dachary, Artisan Logiciel Libre
>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RFC: teuthology field in commit messages

[Loic Dachary]

[-- Attachment #1: Type: text/plain, Size: 987 bytes --]



On 29/11/2015 21:08, John Spray wrote:
> On Sat, Nov 28, 2015 at 3:56 PM, Loic Dachary <loic@dachary.org> wrote:
>> Hi Ceph,
>>
>> An optional teuthology field could be added to a commit message like so:
>>
>> teuthology: --suite rbd
>>
>> to state that this commit should be tested with the rbd suite. It could be parsed by bots and humans.
>>
>> It would make it easy and cost effective to run partial teuthology suites automatically on pull requests.
>>
>> What do you think ?
> 
> Hmm, we are usually testing things at the branch/PR level rather than
> on the per-commit level, so it feels a bit strange to have this in the
> commit message.

Indeed. But what is a branch if not the HEAD commit ?

> However, if a system existed that would auto-test things when I put
> something magic in a commit message, I would probably use it!
> 
> John
> 
> 
>>
>> --
>> Loïc Dachary, Artisan Logiciel Libre
>>

-- 
Loïc Dachary, Artisan Logiciel Libre


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: RFC: teuthology field in commit messages

[John Spray]

On Sun, Nov 29, 2015 at 8:25 PM, Loic Dachary <loic@dachary.org> wrote:
>
>
> On 29/11/2015 21:08, John Spray wrote:
>> On Sat, Nov 28, 2015 at 3:56 PM, Loic Dachary <loic@dachary.org> wrote:
>>> Hi Ceph,
>>>
>>> An optional teuthology field could be added to a commit message like so:
>>>
>>> teuthology: --suite rbd
>>>
>>> to state that this commit should be tested with the rbd suite. It could be parsed by bots and humans.
>>>
>>> It would make it easy and cost effective to run partial teuthology suites automatically on pull requests.
>>>
>>> What do you think ?
>>
>> Hmm, we are usually testing things at the branch/PR level rather than
>> on the per-commit level, so it feels a bit strange to have this in the
>> commit message.
>
> Indeed. But what is a branch if not the HEAD commit ?

It's the HEAD commit, and its ancestors.  So in a typical PR (or
branch) there are several commits since the base (i.e. since master),
and perhaps only one of them has a test suite marked on it, or maybe
they have different test suites marked on different commits in the
branch.

It's not necessarily a problem, just something that would need to have
a defined behaviour (maybe when testing a PR collect the "teuthology:"
tags from all commits in PR, and run all the suites mentioned?).

John


>
>> However, if a system existed that would auto-test things when I put
>> something magic in a commit message, I would probably use it!
>>
>> John
>>
>>
>>>
>>> --
>>> Loïc Dachary, Artisan Logiciel Libre
>>>
>
> --
> Loïc Dachary, Artisan Logiciel Libre
>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: RFC: teuthology field in commit messages

[Loic Dachary]

[-- Attachment #1: Type: text/plain, Size: 1956 bytes --]



On 29/11/2015 21:47, John Spray wrote:
> On Sun, Nov 29, 2015 at 8:25 PM, Loic Dachary <loic@dachary.org> wrote:
>>
>>
>> On 29/11/2015 21:08, John Spray wrote:
>>> On Sat, Nov 28, 2015 at 3:56 PM, Loic Dachary <loic@dachary.org> wrote:
>>>> Hi Ceph,
>>>>
>>>> An optional teuthology field could be added to a commit message like so:
>>>>
>>>> teuthology: --suite rbd
>>>>
>>>> to state that this commit should be tested with the rbd suite. It could be parsed by bots and humans.
>>>>
>>>> It would make it easy and cost effective to run partial teuthology suites automatically on pull requests.
>>>>
>>>> What do you think ?
>>>
>>> Hmm, we are usually testing things at the branch/PR level rather than
>>> on the per-commit level, so it feels a bit strange to have this in the
>>> commit message.
>>
>> Indeed. But what is a branch if not the HEAD commit ?
> 
> It's the HEAD commit, and its ancestors.  So in a typical PR (or
> branch) there are several commits since the base (i.e. since master),
> and perhaps only one of them has a test suite marked on it, or maybe
> they have different test suites marked on different commits in the
> branch.
> 
> It's not necessarily a problem, just something that would need to have
> a defined behaviour (maybe when testing a PR collect the "teuthology:"
> tags from all commits in PR, and run all the suites mentioned?).

That's an interesting idea :-) My understanding is that we currently test a PR by scheduling suites on its HEAD. But maybe you sometime schedule suites using a commit that's in the middle of a PR ?

Cheers

>>
>>> However, if a system existed that would auto-test things when I put
>>> something magic in a commit message, I would probably use it!
>>>
>>> John
>>>
>>>
>>>>
>>>> --
>>>> Loïc Dachary, Artisan Logiciel Libre
>>>>
>>
>> --
>> Loïc Dachary, Artisan Logiciel Libre
>>

-- 
Loïc Dachary, Artisan Logiciel Libre


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: RFC: teuthology field in commit messages

[John Spray]

On Sun, Nov 29, 2015 at 9:25 PM, Loic Dachary <loic@dachary.org> wrote:
>
>
> On 29/11/2015 21:47, John Spray wrote:
>> On Sun, Nov 29, 2015 at 8:25 PM, Loic Dachary <loic@dachary.org> wrote:
>>>
>>>
>>> On 29/11/2015 21:08, John Spray wrote:
>>>> On Sat, Nov 28, 2015 at 3:56 PM, Loic Dachary <loic@dachary.org> wrote:
>>>>> Hi Ceph,
>>>>>
>>>>> An optional teuthology field could be added to a commit message like so:
>>>>>
>>>>> teuthology: --suite rbd
>>>>>
>>>>> to state that this commit should be tested with the rbd suite. It could be parsed by bots and humans.
>>>>>
>>>>> It would make it easy and cost effective to run partial teuthology suites automatically on pull requests.
>>>>>
>>>>> What do you think ?
>>>>
>>>> Hmm, we are usually testing things at the branch/PR level rather than
>>>> on the per-commit level, so it feels a bit strange to have this in the
>>>> commit message.
>>>
>>> Indeed. But what is a branch if not the HEAD commit ?
>>
>> It's the HEAD commit, and its ancestors.  So in a typical PR (or
>> branch) there are several commits since the base (i.e. since master),
>> and perhaps only one of them has a test suite marked on it, or maybe
>> they have different test suites marked on different commits in the
>> branch.
>>
>> It's not necessarily a problem, just something that would need to have
>> a defined behaviour (maybe when testing a PR collect the "teuthology:"
>> tags from all commits in PR, and run all the suites mentioned?).
>
> That's an interesting idea :-) My understanding is that we currently test a PR by scheduling suites on its HEAD. But maybe you sometime schedule suites using a commit that's in the middle of a PR ?

I think I've made this too complicated...

What I meant was that while one would schedule suites against the HEAD
of the PR, that might not be the same commit that has the logical
testing information in.  For example, I might have main commit that
has the "Fixes: " and "teuthology: " tags, and then a second commit
(that would be HEAD) which e.g. tweaks a unit test.  It would be weird
if I had to put the teuthology: tag on the unit test commit rather
than the functional test, so I guess it would make sense to look at
the teuthology: tags from all the commits in a PR when scheduling it.

John

Re: RFC: teuthology field in commit messages

[Loic Dachary]

[-- Attachment #1: Type: text/plain, Size: 2768 bytes --]



On 29/11/2015 23:55, John Spray wrote:
> On Sun, Nov 29, 2015 at 9:25 PM, Loic Dachary <loic@dachary.org> wrote:
>>
>>
>> On 29/11/2015 21:47, John Spray wrote:
>>> On Sun, Nov 29, 2015 at 8:25 PM, Loic Dachary <loic@dachary.org> wrote:
>>>>
>>>>
>>>> On 29/11/2015 21:08, John Spray wrote:
>>>>> On Sat, Nov 28, 2015 at 3:56 PM, Loic Dachary <loic@dachary.org> wrote:
>>>>>> Hi Ceph,
>>>>>>
>>>>>> An optional teuthology field could be added to a commit message like so:
>>>>>>
>>>>>> teuthology: --suite rbd
>>>>>>
>>>>>> to state that this commit should be tested with the rbd suite. It could be parsed by bots and humans.
>>>>>>
>>>>>> It would make it easy and cost effective to run partial teuthology suites automatically on pull requests.
>>>>>>
>>>>>> What do you think ?
>>>>>
>>>>> Hmm, we are usually testing things at the branch/PR level rather than
>>>>> on the per-commit level, so it feels a bit strange to have this in the
>>>>> commit message.
>>>>
>>>> Indeed. But what is a branch if not the HEAD commit ?
>>>
>>> It's the HEAD commit, and its ancestors.  So in a typical PR (or
>>> branch) there are several commits since the base (i.e. since master),
>>> and perhaps only one of them has a test suite marked on it, or maybe
>>> they have different test suites marked on different commits in the
>>> branch.
>>>
>>> It's not necessarily a problem, just something that would need to have
>>> a defined behaviour (maybe when testing a PR collect the "teuthology:"
>>> tags from all commits in PR, and run all the suites mentioned?).
>>
>> That's an interesting idea :-) My understanding is that we currently test a PR by scheduling suites on its HEAD. But maybe you sometime schedule suites using a commit that's in the middle of a PR ?
> 
> I think I've made this too complicated...
> 
> What I meant was that while one would schedule suites against the HEAD
> of the PR, that might not be the same commit that has the logical
> testing information in.  For example, I might have main commit that
> has the "Fixes: " and "teuthology: " tags, and then a second commit
> (that would be HEAD) which e.g. tweaks a unit test.  It would be weird
> if I had to put the teuthology: tag on the unit test commit rather
> than the functional test, so I guess it would make sense to look at
> the teuthology: tags from all the commits in a PR when scheduling it.

Thanks for explaining, it's cristal clear. 

My initial idea of having a teuthology: tag on the top level commit was indeed naive and wrong. And looking through all commits and scheduling the suites found on the HEAD as you suggest reflect what we manually do and sound right :-) 

Cheers

-- 
Loïc Dachary, Artisan Logiciel Libre


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: RFC: teuthology field in commit messages

[Gregory Farnum]

On Sun, Nov 29, 2015 at 6:15 PM, Loic Dachary <loic@dachary.org> wrote:
>
>
> On 29/11/2015 23:55, John Spray wrote:
>> On Sun, Nov 29, 2015 at 9:25 PM, Loic Dachary <loic@dachary.org> wrote:
>>>
>>>
>>> On 29/11/2015 21:47, John Spray wrote:
>>>> On Sun, Nov 29, 2015 at 8:25 PM, Loic Dachary <loic@dachary.org> wrote:
>>>>>
>>>>>
>>>>> On 29/11/2015 21:08, John Spray wrote:
>>>>>> On Sat, Nov 28, 2015 at 3:56 PM, Loic Dachary <loic@dachary.org> wrote:
>>>>>>> Hi Ceph,
>>>>>>>
>>>>>>> An optional teuthology field could be added to a commit message like so:
>>>>>>>
>>>>>>> teuthology: --suite rbd
>>>>>>>
>>>>>>> to state that this commit should be tested with the rbd suite. It could be parsed by bots and humans.
>>>>>>>
>>>>>>> It would make it easy and cost effective to run partial teuthology suites automatically on pull requests.
>>>>>>>
>>>>>>> What do you think ?
>>>>>>
>>>>>> Hmm, we are usually testing things at the branch/PR level rather than
>>>>>> on the per-commit level, so it feels a bit strange to have this in the
>>>>>> commit message.
>>>>>
>>>>> Indeed. But what is a branch if not the HEAD commit ?
>>>>
>>>> It's the HEAD commit, and its ancestors.  So in a typical PR (or
>>>> branch) there are several commits since the base (i.e. since master),
>>>> and perhaps only one of them has a test suite marked on it, or maybe
>>>> they have different test suites marked on different commits in the
>>>> branch.
>>>>
>>>> It's not necessarily a problem, just something that would need to have
>>>> a defined behaviour (maybe when testing a PR collect the "teuthology:"
>>>> tags from all commits in PR, and run all the suites mentioned?).
>>>
>>> That's an interesting idea :-) My understanding is that we currently test a PR by scheduling suites on its HEAD. But maybe you sometime schedule suites using a commit that's in the middle of a PR ?
>>
>> I think I've made this too complicated...
>>
>> What I meant was that while one would schedule suites against the HEAD
>> of the PR, that might not be the same commit that has the logical
>> testing information in.  For example, I might have main commit that
>> has the "Fixes: " and "teuthology: " tags, and then a second commit
>> (that would be HEAD) which e.g. tweaks a unit test.  It would be weird
>> if I had to put the teuthology: tag on the unit test commit rather
>> than the functional test, so I guess it would make sense to look at
>> the teuthology: tags from all the commits in a PR when scheduling it.
>
> Thanks for explaining, it's cristal clear.
>
> My initial idea of having a teuthology: tag on the top level commit was indeed naive and wrong. And looking through all commits and scheduling the suites found on the HEAD as you suggest reflect what we manually do and sound right :-)

And at that point I'm not sure what the point is of having the
"teuthology:" tags in the commit instead of in the PR body. I like
automating things but I think testing will always require enough user
discretion that having to look at the original PR and think about what
changes are being made, rather than just scraping some text out of a
commit, is the appropriate thing to do.
-Greg