On Wed, Feb 25, 2015 at 11:20:45PM +0100, Lukas Slebodnik wrote:
On (25/02/15 16:57), Jakub Hrozek wrote:
On Wed, Feb 25, 2015 at 03:17:54PM +0100, Pavel Březina wrote:
On 02/25/2015 03:10 PM, Lukas Slebodnik wrote:
On (25/02/15 10:55), Jakub Hrozek wrote:
On Wed, Feb 25, 2015 at 10:14:18AM +0100, Jakub Hrozek wrote:
On Wed, Feb 25, 2015 at 09:51:24AM +0100, Lukas Slebodnik wrote: >On (25/02/15 08:33), Jakub Hrozek wrote: >>On Tue, Feb 24, 2015 at 09:16:09PM +0100, Lukas Slebodnik wrote: >>>On (24/02/15 19:52), Jakub Hrozek wrote: >>>>On Tue, Feb 24, 2015 at 05:52:26PM +0100, Lukas Slebodnik wrote: >>>>>On (24/02/15 16:55), Jakub Hrozek wrote: >>>>>>Hi, >>>>>> >>>>>>the attached two patches add a way to detect pre-1.0 cmocka and adds >>>>>>compatible definitions in the first patch and uses them to convert a >>>>>>single unit test. >>>>>> >>>>>>I found the approach ugly myself, so much that I'm considering converting >>>>>>all cmocka-based tests to cmocka-1.0 and don't compile the cmocka tests at >>>>>>all unless 1.0 or later is present on the system. It's not functionality >>>>>>after all, "just" tests and for CI we could add cmocka-1.0 to the CI >>>>>>system ourselves.. >>>>>> >>>>>>Opinions? >>>>>+1 for patch. >>>> >>>>Does +1 for the patch also mean -1 for the proposal to *only* support >>>>cmocka-1.0 and later? >>>> >>>>>I already see deprecated warnings. cmocka 1.0 is in f21 updates-testing >>>> >>>>Yes, btw Andreas would update libcmocka on all releases, including >>>>private RHEL buildroots. So the "only" systems running pre-1.0 cmocka >>>>would be non-RH distributions. >>>> >>>>For instance Ubuntu contains 0.4.. >>>cmocka is optional dependency but we try to run CI build on debian testing. >>>Debian testing (Jessie) is frozen since 2014-Oct-05. So we will need to wait >>>for next debian release to have cmocka-1.0 in (next) Debian testing) >>> >>>If we agree we disable cmocka tests in our CI on debian >>>I'm fine with support *only* cmocka-1.0 and later. >> >>I was proposing to build cmocka-1.0 from source on the Debian CI >>machines. >I haven't seen this proposal yet :-) > >IMHO, it's reasonable compromise. >If Nikolai agrees let's go with cmocka-1.0+ way
Does Nikolai agree? :-)
btw what about the stable branches, do we just remove the test when backporting patches with new tests? I think that would be the easiest way..
Alternatively, we could add the new tests to a new block HAVE_CMOCKA_1_0 in Makefile.am -- that would bring some work when backporting patches, but we wouldn't have to diverge or change the code itself, only the Makefile.am hunk, where the conflict would be minimal.
I would prefer adding strict requirements to cmocka-1.0 into configure and if it is not detected then cmocka test will not be executed.
We can ignore(disable) deprecated warning in stable branches:-)
LS
+1
I'm not sure we understood each other, I was specifically asking about stabe branch. In master, we would add strict requirements for cmocka 1.0+, convert all tests there and don't run any tests if cmocka 1.0+ is not found.
Agree with strict requirement Agree with converting tests.
In sssd-1-12, we would keep the existing tests untouched and ignore the deprecation warnings. But what if someone submits a patch that needs to be included in sssd-1-12, too but adds a test that is written using cmocka-1.0 API? We could either: a) backport the patch without the test b) backport the patch as-is, but add the Makefile.am part of the patch into a new block that gets executed only if cmocka 1.0 is available. Currently we only have a global HAVE_CMOCKA if-endif.
c) - add strict requirement for cmocka 1.0+ - do not convert tests - ignore warnings caused by deprecated function in cmocka 1.0
Result: * change in stable branch will be small * there will not be problem with backporting new tests * makefile will not be poluted with another if else block * if cmocka-1.0+ is not available then cmocka tests will not be executed
Distributions: arch - does not run tests at all opensuse - does not run tests at all gentoo - does not run cmocka test debian - cmocka tests are disabled (due to failueres on some arch) [1] ubuntu - cmocka tests are disabled (due to failueres on some arch) fedora - cmocka-1.0 will be available epel{6,7} - cmocka-1.0 will be available
OK, I didn't propose this change because it would /decrease/ the test coverage. But seeing the summary above, it does seem like the best solution.
Thanks.