Lukas Slebodnik wrote:
> On (03/12/15 09:59), Rob Crittenden wrote:
>> Lukas Slebodnik wrote:
>>> On (02/12/15 13:14), Rob Crittenden wrote:
>>>> Is it still mandatory that tests pass the unit tests before acceptance?
>>> Unit test could be executed as part of "%check" phase in spec files.
>>> I recently added C-base unit tests there.
>>>
>>> I was not bale to run "make tests" there because many tests failed.
>>> If they require real IPA server for execution ( or lite server)
>>> then they are not unit test but integration tests.
>>> One solution would be to skip them or to usw cwrap[1] + lite server.
>>> So it can be run also in mock/koji which has many restrictions.
>>
>> It would represent quite a lot of work but it may be a good idea to
>> investigate. Ipsilon uses cwrap for its tests so some of the
>> configuration can be gleaned from that.
>>
>> I would definitely be opposed to this as part of the freeipa.spec in the
>    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> What do you mean by this part?
> 
> Did you mean "running make tests" in spec file?
> If yes, could you elaborate why it's not good idea?
> many projects run tests in "%check"
> sssd, certmonger, glibc ...
> 
> Currently only C-based test are executed. And I added it only recently.

Because it would be overkill during development. The expectation is that
developers and reviewers run the tests before submission/acceptance. If
they fail to do that then it will be obvious.

>> git tree. In Fedora it might help find problems when rawhide becomes
>> Fedora.next so it would provide some value there.
>>
>>>
>>> Also lint should be also part of "%check" phase and not part of
>>> ordinary build.
>>> BTW I could not see a lint[2] in fedora build at all. So I'm not sure
>>> if it is executed with upstream spec file.
>>
>> It isn't there because the expectation is that lint already passes as
>> part of the release process. I don't see the value on running lint on
>> release tarballs.
>>
> That's just an expectation. It needn't be true. Your initial mail was about
> stricter review process. And automating things is best way how to
> enforce it. So reviewer would just build rpms and if "%check" phase
> will not pass then he will not continue with review.
> If it will be part of "%check" then you can use mock and easily ensure
> that test passes on stable fedora and fedora rawhide (and maybe centOS)

By the time downstream gets a tarball it is too late to fix lint errors.
If upstream is doing a release with lint errors then there is something
deeply wrong with the release process. If someone wants to add it to the
downstream spec files I'm not going to complain, I just find it
extremely unlikely that it will provide any value, ever.

I guess the question to ask is: since implementing lint in the dev
process has a release ever gone out where things failed that would have
been caught? I'm pretty sure the answer is no.

>> I also want to keep the focus on the reviewer's responsibility to ensure
>> that patches do what they are supposed to and don't break things.
>>
> yes, but it does not mean that we cannot simplify/automate reviewer's tasks.
> IMHO, as much as possible should be done in spec file; "%check".

It would be grossly inefficient to run the tests with every make rpms.
Are you at all familiar with the IPA build and install process? It is
usually build here and test over there over and over again. Running the
tests with each iterative build would be horrible. It is sometimes bad
enough waiting for the lint to finish.

If someone wants to setup a Jenkins service to automate testing then
fine. It just needs to be done in some way, and it clearly isn't on a
regular basis and it is easily solved without inventing a whole new CI.

rob

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to