This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
--mJnleTaxCN8S70NxSM9ruh6OHmeqScDoW
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
On 07/20/2013 08:53 PM, Alon Bar-Lev wrote:
=20
=20
----- Original Message -----
> From: "Eyal Edri" <eedri(a)redhat.com>
> To: "Alon Bar-Lev" <alonbl(a)redhat.com>
> Cc: "infra" <infra(a)ovirt.org>, "engine-devel"
<engine-devel(a)ovirt.org>=
, "Fabian Deutsch"
<fabiand(a)redhat.com>
> Sent: Saturday, July 20, 2013 9:41:56 PM
> Subject: Re: [Engine-devel] Proposal for new commit msg design for eng=
ine
commits
>
> This change to commit template has nothing to do with CI.
> it's a change that should reflect updated components relevance to the =
commit
> code.
=20
This commit template is mostly invalid, as touching more than one 'subs=
ystem' is possible, and has not enough granularity.
I suggest using a tag at the end with some extra syntax, like:
Components: core, storage, db
Components: all
Components: all, !core, !db
=20
For example, database change should trigger what?
All the jobs that are tagged for
that component (db upgrades I suppose).
And if the changes affect storage components then also storage, if the
changes affect others then those others too.
Infra change should trigger what?
The same, all the jobs that
are tagged as infra.
A change of both user interface and command should trigger what?
All the jobs tagged by user interface and/or command.
So you end up with:
=20
userportal: storage: core: db: some message
As I suggested before, I think it's better if you end with a commit
message like:
Some message
Components: userportal, storage, core, db
Actually it can be easily done with a tag in the gerrit comment instead
of the commit message.
=20
Just to make who happy?
The developer, the qe, the ci and the infra people. This mechanism is to
avoid running all the tests all the time. Of course there are some times
when all the tests should be run to make sure nothing else changed, but
most times you just need to run part of them to make sure you did not
break something obvious.
=20
And maybe there are 50 tests of network, and you need only 5 of them fo=
r the
specific change, how do you mark it, so now a developer need to kno=
w any test? what if you add one tomorrow which is relevant to a similar c=
hange? how do you inform the developer that now he needs 6?
As I said before, what the developer specifies is not a list of tests,
but a list of components, that qe has to map to different sets of tests
that can change with time. So specifying webadmin will run all the tests
in that group, that might be only one, or 100, and might be
increasing/decreasing with time transparently for the developer. Adding
a new component is not common and there's no need to do it so frequently.=
=20
Why should it be the developer responsibility and not the quality ensur=
ing
engineer responsibility to determine which tests should run and when?=
Of course it's the responsibility of the qe engineer to determine when
and which tests should be run. But this is meant to be a new tool for
the developer not a substitute for the full qe tests, so the developer
can easily make sure that he's changes do not break anything obvious
before starting the real tests (that will take more time and resources).
The developer just adds some metadata so the qe engineer can decide
which tests to run per patch, so it's on qe's hand in the end to decide
if ignore or not the metadata and which tests to run.
=20
As far as this template was not actually used for anything but humans, =
it was not
that important, but once you formalize it as an interface, I s=
tep forward and state that the subject line is not the right tool for the=
task at hand (or any for this matter).
I agree with that, I think that it should be a tag similar to Change-Id,
at the end of the commit message.
=20
The fact that you have in each commit are the sources that are modified=
, all the
other data is just plain noise. From the sources that are modif=
ied you should be able to derive a test plan with high chance that this t=
est program will be correct. Human intervention should be possible by ord=
ering special tests that are outside of the standard policy, for cases in=
which the standard policy of deriving tests from sources is too narrow.
That's just not true. The sources are complicated enough to make two
changes in the same file to affect different components. Any reused code
is prone to affect multiple components, making it really hard to
determine by which changed files which tests to run. And if you go down
to the function/class level it's even harder to decide and to maintain.
And of course it's not human error free, as the metadata in the
files/directories is defined and maintained by a human. And in my
opinion is a lot harder to implement and maintain, and a lot less agile,
and does not get rid of the human factor.
=20
Regards,
Alon
=20
=20
>
> Nevertheless, i have no problems with your suggestions for metadata pe=
r
> directory to map all ovirt code.
> any suggestion how to push it forward?
>
> Eyal.
>
> ----- Original Message -----
>> From: "Alon Bar-Lev" <alonbl(a)redhat.com>
>> To: "Eyal Edri" <eedri(a)redhat.com>
>> Cc: "infra" <infra(a)ovirt.org>, "engine-devel"
<engine-devel(a)ovirt.org=
,
>> "Fabian Deutsch" <fabiand(a)redhat.com>
>> Sent: Saturday, July 20, 2013 9:34:13 PM
>> Subject: Re: [Engine-devel] Proposal for new commit msg design for en=
gine
>> commits
>>
>>
>>
>> ----- Original Message -----
>>> From: "Eyal Edri" <eedri(a)redhat.com>
>>> To: "infra" <infra(a)ovirt.org>
>>> Cc: "engine-devel" <engine-devel(a)ovirt.org>, "Fabian
Deutsch"
>>> <fabiand(a)redhat.com>
>>> Sent: Saturday, July 20, 2013 8:34:28 PM
>>> Subject: Re: [Engine-devel] Proposal for new commit msg design for e=
ngine
>>> commits
>>>
>>> OK, Rome wasn't built in a day.
>>>
>>> To move things forward,
>>> I propose we'll just improve current commit header template to inclu=
de
>>> more
>>> relevant code
>>> areas [1], and start looking into mapping all code to the relevant
>>> components
>>> (either via renaming folders, adding a metadata file under each fold=
er
>>> mapping the files/classnames/directory names or using
automated tool=
s
>>> like
>>> sonar)
>>
>> Again, and I am sorry, but I disagree of any relationship between com=
mit
>> message and CI.
>>
>> It will be simple to add metadata to sources, and have CI run tests b=
ased
>> on
>> actual source change thus probable impact, this way we won't be expos=
ed to
>> human errors, nor make commit message unusable for actual
history.
>>
>> All we need is someone to take ownership of the task of adding metada=
ta
to
>> source tree.
>>
>> As I proposed this can be either within every source using special
>> signature,
>> or can be in a directory at special file, for example .ovirt-metadata=
,
and
>> have the mapping between source component to relevant tests
at a simp=
le
>> text
>> file at source root.
>>
>> Regards,
>> Alon Bar-Lev.
>>
>>>
>>> [1] instead of <core | restapi | tools | history | eng=
ine |
>>> userportal | webadmin>
>>> change to something like <core | restapi | tools | userportal |
>>> webadmin
>>> | network | storage | virt | packaging>
>>>
>>> Eyal.
>>>
>>> ----- Original Message -----
>>>> From: "Moran Goldboim" <mgoldboi(a)redhat.com>
>>>> To: "Eyal Edri" <eedri(a)redhat.com>
>>>> Cc: "Fabian Deutsch" <fabiand(a)redhat.com>,
"engine-devel"
>>>> <engine-devel(a)ovirt.org>, "infra"
<infra(a)ovirt.org>
>>>> Sent: Sunday, July 14, 2013 6:07:05 PM
>>>> Subject: Re: [Engine-devel] Proposal for new commit msg design for
>>>> engine
>>>> commits
>>>>
>>>> On 07/11/2013 11:57 AM, Eyal Edri wrote:
>>>>>
>>>>> ----- Original Message -----
>>>>>> From: "Fabian Deutsch" <fabiand(a)redhat.com>
>>>>>> To: "Eyal Edri" <eedri(a)redhat.com>
>>>>>> Cc: "Alon Bar-Lev" <alonbl(a)redhat.com>,
"engine-devel"
>>>>>> <engine-devel(a)ovirt.org>, "infra"
<infra(a)ovirt.org>
>>>>>> Sent: Thursday, July 11, 2013 11:41:24 AM
>>>>>> Subject: Re: [Engine-devel] Proposal for new commit msg design
fo=
r
>>>>>> engine
>>>>>> commits
>>>>>>
>>>>>> Am Mittwoch, den 10.07.2013, 15:27 -0400 schrieb Eyal Edri:
>>>>>>> ----- Original Message -----
>>>>>>>> From: "Fabian Deutsch"
<fabiand(a)redhat.com>
>>>>>>>> To: "Alon Bar-Lev" <alonbl(a)redhat.com>
>>>>>>>> Cc: "engine-devel"
<engine-devel(a)ovirt.org>, "infra"
>>>>>>>> <infra(a)ovirt.org>
>>>>>>>> Sent: Tuesday, July 9, 2013 3:54:06 PM
>>>>>>>> Subject: Re: [Engine-devel] Proposal for new commit msg
design =
for
>>>>>>>> engine
>>>>>>>> commits
>>>>>>>>
>>>>>>>> Am Dienstag, den 09.07.2013, 08:49 -0400 schrieb Alon
Bar-Lev:
>>>>>>>>>
>>>>>>>>> ----- Original Message -----
>>>>>>>>>> From: "Yair Zaslavsky"
<yzaslavs(a)redhat.com>
>>>>>>>>>> To: "Alon Bar-Lev"
<alonbl(a)redhat.com>
>>>>>>>>>> Cc: "Eyal Edri"
<eedri(a)redhat.com>, "engine-devel"
>>>>>>>>> <engine-devel(a)ovirt.org>, "infra"
<infra(a)ovirt.org>
>>>>>>>>>> Sent: Tuesday, July 9, 2013 3:42:24 PM
>>>>>>>>>> Subject: Re: [Engine-devel] Proposal for new
commit msg desig=
n
>>>>>>>>>> for
>>>>>>>>> engine commits
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> ----- Original Message -----
>>>>>>>>>>> From: "Alon Bar-Lev"
<alonbl(a)redhat.com>
>>>>>>>>>>> To: "Eyal Edri"
<eedri(a)redhat.com>
>>>>>>>>>>> Cc: "engine-devel"
<engine-devel(a)ovirt.org>, "infra"
>>>>>>>>> <infra(a)ovirt.org>
>>>>>>>>>>> Sent: Tuesday, July 9, 2013 3:33:57 PM
>>>>>>>>>>> Subject: Re: [Engine-devel] Proposal for new
commit msg desi=
gn
>>>>>>>>>>> for
>>>>>>>>> engine
>>>>>>>>>>> commits
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> ----- Original Message -----
>>>>>>>>>>>> From: "Eyal Edri"
<eedri(a)redhat.com>
>>>>>>>>>>>> To: "engine-devel"
<engine-devel(a)ovirt.org>
>>>>>>>>>>>> Cc: "infra"
<infra(a)ovirt.org>
>>>>>>>>>>>> Sent: Tuesday, July 9, 2013 12:38:51 PM
>>>>>>>>>>>> Subject: Proposal for new commit msg
design for engine comm=
its
>>>>>>>>>>>>
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> You all probably know and familiar with
'ovirt-engine' git
>>>>>>>>>>>> hook
>>>>>>>>> for
>>>>>>>>>>>> commit
>>>>>>>>>>>> msg template [1].
>>>>>>>>>>>> this helps understand the general area of
the patch in the
>>>>>>>>> project but it
>>>>>>>>>>>> lacks additional info that might
>>>>>>>>>>>> be valuable for scaling automatic tests
in Jenkins CI.
>>>>>>>>>>>>
>>>>>>>>>>>> Let me explain:
>>>>>>>>>>>>
>>>>>>>>>>>> Infra team is working hard on expanding
oVirt CI
>>>>>>>>>>>> infrastructure
>>>>>>>>> and
>>>>>>>>>>>> adding
>>>>>>>>>>>> more tests in jenkins (per
commit/patch).
>>>>>>>>>>>> Adding important meta-data per patch can
significatly impro=
ve
>>>>>>>>> the ability
>>>>>>>>>>>> to
>>>>>>>>>>>> run specific tests for each
patch/commit,
>>>>>>>>>>>> and not waste valuable resources on
Jenkins jobs that are n=
ot
>>>>>>>>> relevant to
>>>>>>>>>>>> the
>>>>>>>>>>>> code in the patch.
>>>>>>>>>>>>
>>>>>>>>>>>> So the idea is to add/expand current
metadata per patch, in=
>>>>>>>>>>>> the
>>>>>>>>> form of:
>>>>>>>>>>>> (either)
>>>>>>>>>>>> 1. expanding current header template to
include more data=
>>>>>>>>>>>> like
>>>>>>>>> 'network'
>>>>>>>>>>>> ,
>>>>>>>>>>>> 'setup', 'tools',
'virt'
>>>>>>>>>>> Please do not expand header, it is too short
anyway.
>>>>>>>>>>>
>>>>>>>>>>>> 2. adding a new label with relevant
tags for the patch,
>>>>>>>>>>>> called
>>>>>>>>> e.g
>>>>>>>>>>>> 'METADATA: network, rest,
virt'
>>>>>>>>>>> Having:
>>>>>>>>>>>
>>>>>>>>>>> CI-Tests: xxx
>>>>>>>>>>> CI-Tests: yyy
>>>>>>>>>>> CI-Tests: zzz
>>>>>>>>>>>
>>>>>>>>>>> Is much better.
>>>>>>>>>> I'm not sure we should have CI-Test - as we
might use this fo=
r
>>>>>>>>> something else
>>>>>>>>>> besides CI.
>>>>>>>>>> Region_of_Interest as Dan suggests sounds better
IMHO.
>>>>>>>>> I don't care how this is to be called.
>>>>>>>>> However, I do not think that commit message is the
place for
>>>>>>>>> instructing CI to do anything.
>>>>>>>>> Commit message stays for good, it should contain
information t=
hat
>>>>>>>>> is
>>>>>>>>> required a year from now.
>>>>>>>>> It has nothing to do with tests and such.
>>>>>>>> I agree with Alon here that the Ci informations don't
belong in=
>>>>>>>> the
>>>>>>>> commit msg.
>>>>>>>> My opinion is that a testcase should know what it covers.
This
>>>>>>>> information from the testcase can then be used by any
party to
>>>>>>>> determin
>>>>>>>> if the testcase should be run on a specific commit (which
yield=
s
>>>>>>>> informations about the changed paths,
files, owner, author, etc=
>>>>>>>> ...
>>>>>>>> which might be valuable).
>>>>>>> i think you're missing the point here.
>>>>>>> can you explain how do you propose a test case will know
"what i=
t
>>>>>>> covers"?
>>>>>>>
>>>>>>> let's take an example:
>>>>>>> let's say a new commit comes from ovirt-engine:
>>>>>>>
http://gerrit.ovirt.org/#/c/16668/
>>>>>>> commit msg: "core: Use images instead of volumes at CDA
message"=
=2E
>>>>>>>
>>>>>>> now you have 1000 test cases (could be system or functional
test=
).
>>>>>>> (let's assume that your infra
can't support running 1000 tests p=
er
>>>>>>> patch/commit).
>>>>>>>
>>>>>>> Some of these test suits checks network flow, some virt
>>>>>>> (migration/template
>>>>>>> for e.g), some host install, others storage flows and so
on... )=
=2E
>>>>>>> you have one repo to clone (ovirt-engine,
let's keep vdsm a side=
>>>>>>> for
>>>>>>> a
>>>>>>> min), and to compile the project from for the tests.
>>>>>>>
>>>>>>> now given this scenario, please explain how will you know
which
>>>>>>> test
>>>>>>> from
>>>>>>> the 1000 you have you'll run on it.
>>>>>>> do you believe that according to the author/path/filename
you'll=
>>>>>>> know
>>>>>>> if
>>>>>>> that patch involves storage or virt scenario?
>>>>>> Hey Eyal,
>>>>>>
>>>>>> Yes - I would at least give it a try to decide automagically
what=
>>>>>> tests
>>>>>> to run by looking at the change.
>>>>>> The main motivation for this is to not add another things which
t=
he
>>>>>> committer needs to take care of and IMO we
humans tend to fail (a=
t
>>>>>> some
>>>>>> point) on those boring tasks like adding correct metadata (let
it=
be
>>>>>> a
>>>>>> typo or just not adding the correct testsuites/topis to be run).
>>>>> this process can be fully automatic via gerrit hooks &
templates:
>>>>>
>>>>> typos or mistakes can be easily handles by gerrit hooks to help th=
e
>>>>> committer fix the tags.
>>>>> as mentions previously, this logic can be done by the project
>>>>> maintainer
>>>>> and enforced by a template or hook.
>>>>>
>>>>> so for example - if someone writes a patch with patch header
>>>>> "webadmin:...." ,
>>>>> then the tags he'll get to choose from are only relevant to
ui/ux.=
>>>>>
>>>>> so the only task a committer will have to do is to verify the defa=
ult
>>>>> tags
>>>>> are relevant.
>>>>>
>>>>> i don't believe this is too much to ask for, considering the
huge
>>>>> benefit
>>>>> that we'll get
>>>>> (stable code, less bugs, less ci breakage, mapping of specific cod=
e
>>>>> to
>>>>> relevant topic, statistics.. etc..)
>>>>>
>>>>>> But let's get back to your example.
>>>>>> Basically we can use the path and filename to determin what
>>>>>> testsuite
>>>>>> to
>>>>>> run.
>>>>>> Testsuites could be bound to paths and/or filenames and/or
regexe=
s
>>>>>> being
>>>>>> matched against the full filename.
>>>>>> Another approach would be to use a java package dependency tree
t=
o
>>>>>> determine which classes are directly and
indirectly affected by a=
>>>>>> change. This information can then be used to
also build a set of
>>>>>> testsuites to be run. For example:
>>>>>> World uses Ocean uses Wale uses Cell - if Wale changes, we'll
sur=
ely
>>>>>> want to run the testsuites assigned to the
classes higher up in t=
he
>>>>>> dependency chain (World and Ocean).
>>>>>>
>>>>>> For the concrete example above: Maybe there is a spell checker
>>>>>> testcase
>>>>>> which could be bound to the filename glob pattern *.properties.
>>>>>>
>>>>>>> i don't think there's an alternative to a metadata to
assist
>>>>>>> mapping
>>>>>>> the
>>>>>>> patch to a relevant "topic" in the code.
>>>>>>> whether it exists as a git note or a label in the commit,
that's=
>>>>>>> another
>>>>>>> matter and probably less important.
>>>>>> The idea is to use the path/filename and dependency tree
informat=
ion
>>>>>> to
>>>>>> model these topics. Example:
>>>>>> WaterTestsuite(Topic):
>>>>>> regex_in_change: .*\.fish
>>>>>> glob_in_change: src/classes/ocean/*.java
>>>>>> path_in_change: src/classes/water.java
>>>>>> change_affects_depency_of: WaterClass
>>>>> I'm not familiar that much with the names of the classes and
>>>>> filenames,
>>>>> but
>>>>> it sounds to me like an error prone process
>>>>> and very complex to start going through all the classes and file
>>>>> names
>>>>> and
>>>>> mapping them to a certain project/component.
>>>>> sounds like we'll have to enforce a naming convention for every
ne=
w
>>>>> file/path/class name that won't break that
magic
>>>>> detection.
>>>>>
>>>>> sure there are exceptions that will work probably, like
"anything
>>>>> under
>>>>> packaging/, should trigger the 'engine-setup' or
'engine-upgrade'
>>>>> tests,
>>>>> but imo, it is not so easy with other components.
>>>>>
>>>>> if something will help, it will be splitting up ovirt-engine into
>>>>> subject
>>>>> projects (different git)
>>>>>
>>>>> Eyal.
>>>>
>>>> I think some valid points were raised in this thread, and I feel we=
all
>>>> agree regarding the need for such a mechanism.
>>>> regarding mapping of different areas in the code using metadata, i
>>>> think
>>>> this approach worth trying, it'll increase ownership and area of
>>>> responsibility within our code and hopefully provide us the
>>>> functionality we are looking for.
>>>> we can start doing the obvious mapping, after-which the responsibil=
ity
>>>> of each team/maintainer to assign a file to a person
and define the=
>>>> specific functional areas in it.
>>>>
>>>> Moran.
>>>>
>>>>>
>>>>>> But surely labels or meta-data in the commit msg are quicker to
>>>>>> implement.
>>>>>>
>>>>>> - fabian
>>>>>>
>>>>>>> eyal.
>>>>>>>
>>>>>>>> - fabian
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> Engine-devel mailing list
>>>>>>>> Engine-devel(a)ovirt.org
>>>>>>>>
http://lists.ovirt.org/mailman/listinfo/engine-devel
>>>>>>>>
>>>>>>
>>>>>>
>>>>> _______________________________________________
>>>>> Infra mailing list
>>>>> Infra(a)ovirt.org
>>>>>
http://lists.ovirt.org/mailman/listinfo/infra
>>>>
>>>>
>>> _______________________________________________
>>> Engine-devel mailing list
>>> Engine-devel(a)ovirt.org
>>>
http://lists.ovirt.org/mailman/listinfo/engine-devel
>>>
>>
>
_______________________________________________
Infra mailing list
Infra(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/infra
=20
--=20
David Caro
Red Hat Czech s.r.o.
Continuous Integration Engineer - EMEA ENG Virtualization R&D
Tel.: +420 532 294 605
Email: dcaro(a)redhat.com
Web:
www.cz.redhat.com
Red Hat Czech s.r.o., Purky=C5=88ova 99/71, 612 45, Brno, Czech Republic
RHT Global #: 82-62605
--mJnleTaxCN8S70NxSM9ruh6OHmeqScDoW
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.13 (GNU/Linux)
iQEcBAEBAgAGBQJSFxpfAAoJEEBxx+HSYmnD1rkH/0gE9/aZRADQPB4TMvDTywl0
JSAcdc8T34Mvdv64GnlK2YRIDleKbGxFYzsuYAicfpts8C1ES+B58+t7jFboMGZP
+vYkwSYCz1v/FWsgSnKLTwvHTNdnCD0vawZcVzM/G5cbIplY9IzO6jTnFGaO9KtW
9Uj5pRgBEGoXT8EXoM8iGcnPj5St4MxYiz+ZsNXNkD6ZAzNsOC2MvhVuQh01r6ld
snkGzyu4C4l9ENlWWAMrk9wgdcCeK0ct3wQ7nHY+U6drOBDBjVGcPUV97TY02BJh
Oxx9fNgyZj1PgU4rUcmNqR76qpMpfieSlMDbor8Dy8f/N6qTYGG+0B7kKuVoK+A=
=uv5b
-----END PGP SIGNATURE-----
--mJnleTaxCN8S70NxSM9ruh6OHmeqScDoW--