Comment 17 for bug 1549942

On 07.03.2016 [21:32:04 -0000], Steve Langasek wrote:
> On Mon, Mar 07, 2016 at 07:18:25PM -0000, Nish Aravamudan wrote:
> > > So the new php-imagick upload now passes its testsuite on amd64 and ppc64el,
> > > but shows failures on i386, armhf and s390x
>
> > > http://autopkgtest.ubuntu.com/packages/p/php-imagick/xenial/armhf
> > > http://autopkgtest.ubuntu.com/packages/p/php-imagick/xenial/i386
> > > http://autopkgtest.ubuntu.com/packages/p/php-imagick/xenial/s390x
>
> > > Same test failure on i386 and s390x, and only 1 test failure instead of 20+.
> > > Different failure on armhf (a test timeout - possibly a problem of the
> > > architecture being slow rather than a bug in the code).
>
> > Yeah, I'd think the armhf failure is just an issue with the running time
> > as opposed to a bug. That time limit is hardcoded in the test -- would
> > you want me to propose upstream changing that value based upon the
> > platform? Or just increasing it across the board?
>
> I would say it should get more investigation first. For one thing, the
> error message says "Maximum execution time of 3000 seconds exceeded", but
> 3000 seconds is 50 minutes and the entire autopkgtest run only took 5
> minutes.
>
> For now, the test is overridden for proposed-migration.

Ok that is odd, good point!

> > > Is this a new test? Do you believe this test would fail with the previous
> > > version of php-imagick as well? Should we bypass this test failure for now
> > > to get php7 migrated?
>
> > It is a new test, and is one I fixed in the updated version of
> > imagemagick. It was an architectural bug in imagemagick, actually --
> > where i386 had the correct value for rounding, but all other
> > architectures didn't.
>
> > Ah, this actually goes to the heart of the issue I mentioned earlier.
> > This tests (025-get-color.phpt) tests for different behavior based upon
> > the imagemagick version. In our case, we are running 0x689 (base
> > imagemagick version), but actually it's not upstream 0x689, it's 0x689 +
> > backports, in particular some from 6.9.2 that fix the code being tested
> > here.
>
> > So the test does this check:
>
> > if ($v['versionNumber'] >= 0x692) {
> > //this is the new correct behaviour
> > return (intval(round($someValue, 0, PHP_ROUND_HALF_UP)));
> > }
> > else {
> > //old behaviour had wrong rounding.
> > return (intval(round($someValue, 0, PHP_ROUND_HALF_DOWN)));
> > }
>
> > And executes the else-case, but we have the fixes that are in the
> > if-case.
>
> > I'm not sure what to do about this. Technically, we know in this test on
> > Ubuntu, we should now (on all architectures) be testing the if-case, so
> > we could carry a patch for the test that removes the conditional?
>
> I see. That doesn't look like a very sane test to me; the test ought
> to test that the output is *correct*, not test that the output matches
> some known bug. Yes, I think patching this test to always check for
> the correct value is appropriate.

But what is "correct" to an API consumer? That imagemagick always rounds
up? Then php-imagick will always fail against older version of
imagemagick. And, based upon the tests their CI runs, that is not
acceptable upstream. So they integrate knowledge of imagemagick's
expected behavior ... all in all, not ideal, but it makes some sense to
me. In any case, I'll provide another debdiff to alter the test for
Ubuntu.