Bug 97748 - unit-test to check graphics ...
Summary: unit-test to check graphics ...
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
5.1.0.1 rc
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard:
Keywords: difficultyBeginner, easyHack, skillScript, topicCleanup
: 97749 (view as bug list)
Depends on:
Blocks:
 
Reported: 2016-02-11 13:20 UTC by Michael Meeks
Modified: 2017-02-14 08:58 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments
Rough proposed patch (485 bytes, application/x-perl)
2016-08-29 01:53 UTC, Diana Sedlak
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Meeks 2016-02-11 13:20:34 UTC

    
Comment 1 Michael Meeks 2016-02-11 13:20:54 UTC
*** Bug 97749 has been marked as a duplicate of this bug. ***
Comment 2 Michael Meeks 2016-02-11 17:05:32 UTC
Currently if we add an icon to a theme - we should really add a copy to the Galaxy theme as well (this should be the base of all our theme stacks).

Unfortunately this is not always done - and we end up with missing icons in the galaxy theme.

Icons themes are packed by:

postprocess/CustomTarget_images.mk

if you re-run 'make postprocess' you can re-build the themes which are zip files that we stack up.

The perl script to build those is in:

solenv/bin/packimages.pl

When building the default (galaxy) theme we should fail hard - with a nice warning 'die ("foo");' in the perl - if we are missing any icon =)

Thanks !
Comment 3 jani 2016-06-17 06:52:50 UTC
Changing to assigned
Comment 4 jani 2016-07-18 05:29:43 UTC Comment hidden (obsolete)
Comment 5 Diana Sedlak 2016-07-21 00:54:50 UTC
Yes, I am. I am a little stuck. So this is coded in perl, and the graphics are already in the other themes, but not this one? I have been slowing working on it and will hopefully be done within two or three weeks. I will try and work on it right now.
Comment 6 Diana Sedlak 2016-07-21 01:12:23 UTC
Am installing the dependencies and cloning the repository, will try to work on this and hopefully be done soon, if I'm not done in the next two weeks it can be opened back up, but I hope to get it done. I wish that spelling error hadn't been fixed, would have been much easier!
Comment 7 Diana Sedlak 2016-07-21 05:38:18 UTC
Am looking at the Perl scripts and graphics and will work more tomorrow, will let you know if I need help.
Comment 8 Diana Sedlak 2016-07-29 06:24:58 UTC
Have been looking at the scripts at browsing through opengrok. Am stuck. In line 24 of postprocess/CustomTarget_images.mk in opengrok there is a reference to /icon-themes/industrial. Clearly that is an outdated icon theme as stated. But I also searched through /icon-themes/galaxy and there were a lot of images in opengrok. But based on the title of the bug, what I need to do is write an automated unit test to check for graphics correct? In perl? That should throw an error/exception 'die' if there are missing graphics in the galaxy theme compared to the other themes? I'm a complete beginner, so "re-run 'make postprocess'
 would be something I do in the Linux command line?  Or in a Perl script? I think this is a unit test, correct? I'm fairly confused. So I'm going to ask a coworker for help on this tomorrow.  Will try to connect to the irc over the weekend. I'm determined, but confused.  An example of a similar issue, or unit test would be very helpful, or a step by step. But, this appears to be a unit test, that needs to be written in perl, that checks for missing graphics in the default theme compared to the other themes, correct? So is the perl script in 'solenv/bin/packimages.pl' to rebuild the themes which are stacked zip files? Or is the Perl script in solenv/bin/packimages.pl a unit test?  I was confused by the 'rebuild themes' vs 'unit test'.  In order to solve this bug would I need to use git and gerrit and commit to master? Or attach a perl script to test graphics here in bugzilla?
Comment 9 Michael Meeks 2016-08-03 11:25:50 UTC
Hi Diana,

Thanks for looking at this one =)

> But based on the title of the bug, what I need to do is write an automated
> unit test to check for graphics correct ?

Sure.

> In perl? That should throw an error/exception 'die' if there are missing 
> graphics in the galaxy theme compared to the other themes ?

Sure - so what I'd do is hack solenv/bin/packimages.pl - to ensure that for any image we put into a theme .zip file - we check that the same image is present in the galaxy/ theme - and just die if not with some error / warning =)

> I'm determined, but confused.

No problem =)

> An example of a similar issue, or unit test would be very helpful, or a step by step. But, this appears to be a unit test, that needs to be written in perl,

So - if we can avoid creating a unit test and/or a new makefile, and just make the generic theme packing fail if this criteria is not met with a 'die' that would be wonderful and meet the need.

> So is the perl script in 'solenv/bin/packimages.pl' to rebuild the themes
> which are stacked zip files?

Seems reasonable; read it, play with it and see =) add some printouts there; do:
$ cd postprocess; make

to see what they print. After a few minutes of playing, you'll be the new expert on packimages.pl =) people tend not to remember much about every piece of code they just re-learn it when needed.

Having added this test, and test its failure (just remove a file in galaxy I guess to test), then do:

git diff -u > test.patch

and send test.patch to the mailing list =)

Thanks !
Comment 10 Diana Sedlak 2016-08-19 01:59:08 UTC
Alright thanks. Have been busy the past two weeks but am cracking now! thanks!
Comment 11 Diana Sedlak 2016-08-19 02:06:13 UTC
Thanks! I've been busy the past two weeks but just got cracking now!
Comment 12 Diana Sedlak 2016-08-29 01:53:59 UTC
Created attachment 127060 [details]
Rough proposed patch

After being confused, and determined, I have carefully studied packimages.pl and have come up with a rough patch. Will push to mailing list and test my code more this week. This is just using gedit, but i'm excited to finally be making headway.
Comment 13 Michael Meeks 2016-08-29 02:55:40 UTC
Hi Diana, it looks plausible. The easiest way to send a patch is to do:

git diff > /tmp/foo.patch

and attach that - then we see the change in context etc. =)

Thanks !
Comment 14 Diana Sedlak 2016-09-17 04:11:56 UTC
Thanks for getting back to me quickly, unfortunately I have had my personal life get in the way, and my job, get in the way, these past couple weeks. If I am not able to finish this by Oct. 1st I will reopen it. Do I use logerrit for the patcch? Will work on Git starting Sunday.
Comment 15 jani 2016-10-18 06:23:00 UTC
(In reply to Diana Sedlak from comment #14)
> Thanks for getting back to me quickly, unfortunately I have had my personal
> life get in the way, and my job, get in the way, these past couple weeks. If
> I am not able to finish this by Oct. 1st I will reopen it. Do I use logerrit
> for the patcch? Will work on Git starting Sunday.

A polite ping still working on this patch ? 

Do you need help, then please do not hesitate to contact me (or simply comment on the ticket)
Comment 16 jani 2016-11-18 06:19:34 UTC
Unassigning, seems not to be worked on. If work is still ongoing, then please assign it again.
Comment 17 jani 2016-12-24 08:26:07 UTC Comment hidden (obsolete)
Comment 18 Diana Sedlak 2016-12-29 22:36:28 UTC
No please remove me as the assignee.
Comment 19 Diana Sedlak 2016-12-29 22:37:11 UTC Comment hidden (obsolete)
Comment 20 jani 2016-12-29 23:43:58 UTC
Well that is normally something you should do yourself, and not wait for others to do it.