Comment 16 for bug 1549942

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.

> > 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.