286724 - Charting test images are very very slightly off

Status: RESOLVED FIXED

(Bugzilla :: Testing Suite, defect)

Description

[Marc Schumann [:Wurblzap]]

On my system, the .png images generated by testserver.pl look at first glance
exactly like the known good ones from bug 275705 Bugzilla comes with.

On second glance, the lines in testchart-local.png appear to be a little thinner
than the original, and the red blob in testgd-local.png is very slightly smaller
than the original.

My setup is Win2000 with ActiveState perl 5.8.4; checksetup.pl says this about
my perl modules:
Checking perl modules ...
Checking for       AppConfig (v1.52)   ok: found v1.52
Checking for             CGI (v2.93)   ok: found v3.04
Checking for    Data::Dumper (any)     ok: found v2.121
Checking for    Date::Format (v2.21)   ok: found v2.22
Checking for             DBI (v1.38)   ok: found v1.42
Checking for      DBD::mysql (v2.1010) ok: found v2.9003
Checking for      File::Spec (v0.82)   ok: found v0.87
Checking for      File::Temp (any)     ok: found v0.14
Checking for        Template (v2.08)   ok: found v2.09
Checking for      Text::Wrap (v2001.0131) ok: found v2001.09291
Checking for    Mail::Mailer (v1.65)   ok: found v1.66

The following Perl modules are optional:
Checking for              GD (v1.20)   ok: found v2.06
Checking for     Chart::Base (v1.0)    ok: found v2.2
Checking for     XML::Parser (any)     ok: found v2.34
Checking for       GD::Graph (any)     ok: found v1.39
Checking for GD::Text::Align (any)     ok: found v1
Checking for     PatchReader (v0.9.4)   not found

Comment 1

[Marc Schumann [:Wurblzap]]

Attachment: testchart-local.png

Comment 2

[Marc Schumann [:Wurblzap]]

Attachment: testgd-local.png

Comment 3

[Gervase Markham [:gerv]]

So presumably testserver.pl fails for you?

Gerv

Comment 4

[Marc Schumann [:Wurblzap]]

(In reply to comment #3)
> So presumably testserver.pl fails for you?

Yes; I'm getting the "TEST-WARNING GD library generated a PNG that didn't match
the expected image" and "TEST-WARNING Chart library generated a PNG that didn't
match the expected image" messages.

Fwiw, it works just fine on WinXP with ActiveState perl 5.8.6, with these perl
modules:

Checking perl modules ...
Checking for       AppConfig (v1.52)   ok: found v1.56
Checking for             CGI (v2.93)   ok: found v3.05
Checking for    Data::Dumper (any)     ok: found v2.121_02
Checking for    Date::Format (v2.21)   ok: found v2.22
Checking for             DBI (v1.38)   ok: found v1.46
Checking for      DBD::mysql (v2.1010) ok: found v2.9004
Checking for      File::Spec (v0.82)   ok: found v3.01
Checking for      File::Temp (any)     ok: found v0.14
Checking for        Template (v2.08)   ok: found v2.13
Checking for      Text::Wrap (v2001.0131) ok: found v2001.09292
Checking for    Mail::Mailer (v1.65)   ok: found v1.66

The following Perl modules are optional:
Checking for              GD (v1.20)   ok: found v2.19
Checking for     Chart::Base (v1.0)    ok: found v2.3
Checking for     XML::Parser (any)     ok: found v2.34
Checking for       GD::Graph (any)     ok: found v1.43
Checking for GD::Text::Align (any)     ok: found v1.18
Checking for     PatchReader (v0.9.4)   not found

Comment 5

[Gervase Markham [:gerv]]

Attachment: Patch v.1

Here's a patch which simplifies the images as much as possible to improve
compatibility while still testing what we need to test. Reference images coming
up.

glob - when you review, can you please generate a corresponding GIF? I don't
think I have the ability.

Gerv

Comment 6

[Gervase Markham [:gerv]]

Attachment: New testchart.png

Comment 7

[Gervase Markham [:gerv]]

Attachment: New testgd.png

Comment 8

[:glob ✱]

Comment on attachment 178930 [details] [diff] [review]
Patch v.1

sorry, but i think that a black square isn't a very good test image.

when i first viewed it i thought the attachment was corrupt.

marc -- can you please check if this patch makes the chart look correct for
you?

Comment 9

[Marc Schumann [:Wurblzap]]

(In reply to comment #8)
> marc -- can you please check if this patch makes the chart look correct for
> you?

I don't have any older module versions available any more.

I'd suspect the line in the chart image to be a little too thin if I tried with
an old setup -- that's what caused the original deviation.

I'm starting to think we might want to mark this bug as WONTFIX. The warning
message tells people to compare the generated image with the expected image;
they should do that and be happy with near optical equivalency. No need to be as
nitpicky as I am :)

Comment 10

[Gervase Markham [:gerv]]

I think it's worth simplifying the generated images as far as possible while
still testing the modules in question.

glob: I'd be happy with a red square on a black square (say). What I want to
avoid are corners and curves, which will anti-alias differently on different
platforms and versions.

Gerv

Comment 11

[:glob ✱]

gerv:  ok, bring on the squares :)

Comment 12

[Gervase Markham [:gerv]]

Attachment: Patch v.2

Try this.

Gerv

Comment 13

[Gervase Markham [:gerv]]

Attachment: testchart.png v.2

Comment 14

[Gervase Markham [:gerv]]

Attachment: testgd.png v.2

Comment 15

[:glob ✱]

Comment on attachment 179381 [details] [diff] [review]
Patch v.2

r=glob

i'm still trying to find a system that'll generate gifs.

Comment 16

[Gervase Markham [:gerv]]

glob: well, we can either create a matching GIF or remove the GIF test :-)

Gerv

Comment 17

[Gervase Markham [:gerv]]

glob: ping? :-)

Gerv

Comment 18

[:glob ✱]

nuke the gif.

Comment 19

[Gervase Markham [:gerv]]

OK. Any chance of an updated patch? :-)

Gerv

Comment 20

[:glob ✱]

well, you did the existing patches and the bug is assigned to you ... ;)

Comment 21

[Gervase Markham [:gerv]]

Did I? Doh! OK :-)

Gerv

Comment 22

[Max Kanat-Alexander]

Only the fact that testserver.pl is broken makes this a blocker.

If the fix is trivial, and the problem also happens in 2.20, I think it couldn't hurt to get this into 2.20 also.

Comment 23

[Frédéric Buclin]

gerv, what about this bug? It has a patch with r+ on it but per comment 19, it looks like it requires a new one. And it's a blocker...

Comment 24

[Gervase Markham [:gerv]]

OK, this isn't going to work. I just tried the patch on my laptop and, although the two copies of testgd.png are identical in size and look, they are not identical in terms of bytes. 

I can only assume that some quirk of the PNG encoder has caused them to be encoded a bit differently but, regardless, binary diff isn't going to cut it as a way of seeing if two images differ. I can't immediately think of another way of doing it, short of shipping some pngdiff program, and that doesn't sound like fun.

Any ideas?

Gerv

Comment 25

[:glob ✱]

png's lossless, so comparing the images is pretty simple -- load both images and compare pixel-by-pixel reading from the canvases.

that said, i doubt the effort would be worth it.  i'd be happy if we just generated the test image and told the user to eyeball them.

Comment 26

[Dave Miller [:justdave]]

if testserver.pl passes, then their webserver is serving images...  drop the test image next to the known-good one, generate an HTML page that pulls them both in side by side, and give the user a URL to hit (based on the one they provided on the command line).

Comment 27

[Gervase Markham [:gerv]]

Hmm. Could we split testserver.pl into testserver.pl and testserver.cgi? 

testserver.pl would check that CGIs can actually run, and testserver.cgi would test things like this. As it returns a web page, you could get people to eyeball the results or even use <canvas> as someone suggested.

Gerv

Comment 28

[Marc Schumann [:Wurblzap]]

Can we do this the other way round? I mean, can we tell somehow whether GD does *not* work, by looking at the file or by looking at $image->png? I don't know, maybe the magic number is missing if image generation failed?

Comment 29

[Gervase Markham [:gerv]]

Attachment: Patch v.3

OK, here's another patch. This one:

- Removes GIF testing support
- Switches the test to just check for the PNG header
- Updates the libgd check; we can't rely on libgd-config being present if libgd 
  2.x is installed (for example, it's not on my Ubuntu system) so we don't do 
  the check if we can't find libgd-config.

Checkin of this patch would be accompanied by the CVS removal of the two PNGs in the t/ directory.

Gerv

Comment 30

[:glob ✱]

Comment on attachment 207253 [details] [diff] [review]
Patch v.3

r=glob

most of this looks good, 

>+    if ($filedata =~ /^.PNG/) {
>+        print "TEST-OK $library library generated a good PNG image.\n";

i would prefer to check for the full png signature:
  if ($filedata =~ /^\x89\x50\x4E\x47\x0D\x0A\x1A\x0A/) {

bonus points if you do this on checkin :)

Comment 31

[Frédéric Buclin]

(In reply to comment #29)
> Checkin of this patch would be accompanied by the CVS removal of the two PNGs
> in the t/ directory.


gerv, I suppose t/testchart.gif can be removed too?

Comment 32

[Gervase Markham [:gerv]]

Fixed.

Checking in testserver.pl;
/cvsroot/mozilla/webtools/bugzilla/testserver.pl,v  <--  testserver.pl
new revision: 1.9; previous revision: 1.8
done
Removing t/testchart.png;
/cvsroot/mozilla/webtools/bugzilla/t/testchart.png,v  <--  testchart.png
new revision: delete; previous revision: 1.1
done
Removing t/testchart.gif;
/cvsroot/mozilla/webtools/bugzilla/t/testchart.gif,v  <--  testchart.gif
new revision: delete; previous revision: 1.1
done
Removing t/testgd.png;
/cvsroot/mozilla/webtools/bugzilla/t/testgd.png,v  <--  testgd.png
new revision: delete; previous revision: 1.1
done

Gerv