[SSSD] [PATCH v3] CI: Allow disabling distro-(in)dependent tests

Lukas Slebodnik lslebodn at redhat.com
Thu Sep 18 13:55:47 UTC 2014


On (12/09/14 16:00), Nikolai Kondrashov wrote:
>On 09/12/2014 01:05 PM, Nikolai Kondrashov wrote:
>>On 09/12/2014 12:42 PM, Michal Židek wrote:
>>>I disagree. I think we are not in hurry with this particular feature
>>>(I now mean the running dependent and independent tests separately)
>>>so I would prefer to have proper solution rather than something
>>>we will have to rewrite later anyway.
>>
>>I understand, but distro-dependent/independent test running is not a target in
>>itself. The target is to have the CI servers available for routine use.
>>
>>Currently rigorous test runs typically take around an hour. Once Jakub goes
>>through pending and acknowledged patches and merges them in quick succession,
>>the CI servers become unavailable for most of the day, running rigorous
>>post-commit tests. Having people submit jobs for at least moderate tests will
>>also slow it down.
>>
>>I cannot announce such "service" as available for use. Yet it is important to
>>have such a service.
>>
>>>The current patch would not improve my user experience (I said my
>>>user experience, maybe someone else has different opinion) but
>>>the changes I proposed would improve it significantly. And
>>>this patch has functionality that is subset of the ticket #2425,
>>>so even if it takes more time, it will be better to focus on that
>>>one. When it is ready I will gladly review and test (and use!).
>>
>>I understand. However, distro-dependent/independent test running was never
>>intended for local use, so it cannot improve your current experience, only
>>your potential experience of running CI on the servers (with all benefits that
>>brings).
>>
>>Merging this patch (with "-n" added back, if you'd like) doesn't preclude or
>>hinder my work on adding arbitrary test selection as work on *this* patch is
>>done already. OTOH, it does enable server CI use by the team.
>
>Here is the patch with "-n" added back, in case you decide to ACK it.
>
>Nick

>From d31e708577168f2e485941ce2138c9672bcd8828 Mon Sep 17 00:00:00 2001
>From: Nikolai Kondrashov <Nikolai.Kondrashov at redhat.com>
>Date: Tue, 2 Sep 2014 17:45:22 +0300
>Subject: [PATCH 1/1] CI: Allow disabling distro-(in)dependent tests
>
>Add support for --no-common and --no-distro options to contrib/ci/run.
>
>These options disable running distribution-independent and
>distribution-dependent tests respectively, allowing running common
>distro-independent tests separately.
>
>The mock builds and Clang analyzer run are considered
>distro-independent. The test building, running them under Valgrind,
>distcheck target and collecting and verifying coverage data are
>considered distro-dependent.
>
>This allows CI service to run them on separate dedicated hosts, instead
>of running on all hosts, speeding up overall execution.
>---
I really tried to avoid review this patches. I was expecting Michal will come
to the meeting and explain his concerns, but someone had to ACK this patch.

This is the first time I read the patch. I'm so Sorry I have to reject it.
I know that there was an agreement on meeting that we will push the patch.
But on the meeting, there wasn't anyone who read/test the patch.

1) I disagree that mock is distro independent patch.
   The purpose of mock is to build sssd in clean environment. I gave you two
   options: to use mock or to prepare clean Vm for each test.
   Currently we do not buid sssd in clean enviroment for rhel7,
   fedora-rawhdide and debian. The debian cannot be solved easily, but other
   two options can be solved with mock. I mentioned it few times that we have
   hardcoded mock builds for rhel6 and fedora-20. It means than on rhel7 we run
   mockbuild for rhel6 and fedora-20.
   Why? "The purpose of mock is to build sssd in clean environment", therefore
   on rhel7 we should build with mock just for rhel7.

2) clang analysis is ignored anyway. It will be easier to remove it, if we
   want to speed up building.

3) I don't like solution how you solved the problem.
   You added 6 if-else statements(some of them even nested) for 2 new command
   line options. Sorry, It does not scale and script is much more complicated.

You wanted to speed up process.
So, here is my proposal:
    * run mock build just one. (the same as testing platform)
    * enable tmpfs mock plugin on all platforms.
    * remove clang analysis, because it is ignored anyway.
LS



More information about the sssd-devel mailing list