Add support for if nodes in outputs nodes of grd files

Add support for if nodes in outputs nodes of grd files

Make it possible to enclose <output> nodes in <if> conditional, therefore it becomes possible to generate different files from .grd files depending on the platform.

BUG=none
TEST=IfNodeUnittest.testIffynessWithOutputNodes (in grit)

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=56905

[gfeher, 2010-08-20 12:00:52]

Hi Jói, it's me again. I realized that I need this feature because Mac policy
templates can only be generated on Mac. Could you please review this CL?
Thanks, Gábor

[Jói, 2010-08-20 14:51:40]

Looks groovy, please add unit tests to cover the new logic and the new cases for
the <if> node.

[gfeher, 2010-08-20 17:34:12]

Please check my unit test.
By the way, is there a way to add the directory grit/grit/test to the tree? It
is missing and causes a lot of tests to fail.

On 2010/08/20 14:51:40, Jói wrote:
> Looks groovy, please add unit tests to cover the new logic and the new cases
for
> the <if> node.

[Jói, 2010-08-20 17:57:50]

LGTM

+tony

> By the way, is there a way to add the directory grit/grit/test to the tree?
> It
> is missing and causes a lot of tests to fail.

Yes, it should be possible to add that directory.  It's just fully
missing?  Tony, do you know anything about that?

Cheers,
Jói


On Fri, Aug 20, 2010 at 1:34 PM,  <gfeher@chromium.org> wrote:
> Please check my unit test.
> By the way, is there a way to add the directory grit/grit/test to the tree?
> It
> is missing and causes a lot of tests to fail.
>
> On 2010/08/20 14:51:40, Jói wrote:
>>
>> Looks groovy, please add unit tests to cover the new logic and the new
>> cases
>
> for
>>
>> the <if> node.
>
>
>
> http://codereview.chromium.org/3155033/show
>

[Jói, 2010-08-20 17:58:51]

LGTM

Sorry, meant to send a couple of nits:

http://codereview.chromium.org/3155033/diff/16001/17003
File tools/grit/grit/node/misc_unittest.py (right):

http://codereview.chromium.org/3155033/diff/16001/17003#newcode103
tools/grit/grit/node/misc_unittest.py:103: assert uncond1_output.name ==
'output'
I'd change all the asserts should to self.assertTrue(); I'm not sure if the unit
test output will "look right" if it's an assert rather than a self.assertTrue

http://codereview.chromium.org/3155033/diff/16001/17003#newcode113
tools/grit/grit/node/misc_unittest.py:113: ['uncond1.rc', 'only_fr.adm',
'only_fr.plist', 'doc.html', 'uncond2.adm'])
+80

[tony, 2010-08-20 18:02:19]

On 2010/08/20 17:34:12, gfeher wrote:
> Please check my unit test.
> By the way, is there a way to add the directory grit/grit/test to the tree? It
> is missing and causes a lot of tests to fail.

It's in chrome-internal.  You need to add the src-internal DEPS file to your
.gclient.

[gfeher, 2010-08-20 18:05:06]

Thanks!

http://codereview.chromium.org/3155033/diff/16001/17003
File tools/grit/grit/node/misc_unittest.py (right):

http://codereview.chromium.org/3155033/diff/16001/17003#newcode103
tools/grit/grit/node/misc_unittest.py:103: assert uncond1_output.name ==
'output'
May I also change the asserts in the other unit test in this file?

On 2010/08/20 17:58:51, Jói wrote:
> I'd change all the asserts should to self.assertTrue(); I'm not sure if the
unit
> test output will "look right" if it's an assert rather than a self.assertTrue

[Jói, 2010-08-20 18:08:02]

> May I also change the asserts in the other unit test in this file?

SGTM.  Just make sure the tests still pass before submitting :-)

Cheers,
Jói


On Fri, Aug 20, 2010 at 2:05 PM,  <gfeher@chromium.org> wrote:
> Thanks!
>
>
> http://codereview.chromium.org/3155033/diff/16001/17003
> File tools/grit/grit/node/misc_unittest.py (right):
>
> http://codereview.chromium.org/3155033/diff/16001/17003#newcode103
> tools/grit/grit/node/misc_unittest.py:103: assert uncond1_output.name ==
> 'output'
> May I also change the asserts in the other unit test in this file?
>
> On 2010/08/20 17:58:51, Jói wrote:
>>
>> I'd change all the asserts should to self.assertTrue(); I'm not sure
>
> if the unit
>>
>> test output will "look right" if it's an assert rather than a
>
> self.assertTrue
>
> http://codereview.chromium.org/3155033/show
>

[gfeher, 2010-08-20 18:33:45]

I'll just check what the trybots say and submit...

http://codereview.chromium.org/3155033/diff/16001/17003
File tools/grit/grit/node/misc_unittest.py (right):

http://codereview.chromium.org/3155033/diff/16001/17003#newcode103
tools/grit/grit/node/misc_unittest.py:103: assert uncond1_output.name ==
'output'
On 2010/08/20 18:05:06, gfeher wrote:
> May I also change the asserts in the other unit test in this file?
> 
> On 2010/08/20 17:58:51, Jói wrote:
> > I'd change all the asserts should to self.assertTrue(); I'm not sure if the
> unit
> > test output will "look right" if it's an assert rather than a
self.assertTrue
> 
> 

Done.

http://codereview.chromium.org/3155033/diff/16001/17003#newcode113
tools/grit/grit/node/misc_unittest.py:113: ['uncond1.rc', 'only_fr.adm',
'only_fr.plist', 'doc.html', 'uncond2.adm'])
On 2010/08/20 17:58:51, Jói wrote:
> +80

Done.

[Jói, 2010-08-20 18:35:54]

LGTM

Groovy.

On Fri, Aug 20, 2010 at 2:33 PM,  <gfeher@chromium.org> wrote:
> I'll just check what the trybots say and submit...
>
>
> http://codereview.chromium.org/3155033/diff/16001/17003
> File tools/grit/grit/node/misc_unittest.py (right):
>
> http://codereview.chromium.org/3155033/diff/16001/17003#newcode103
> tools/grit/grit/node/misc_unittest.py:103: assert uncond1_output.name ==
> 'output'
> On 2010/08/20 18:05:06, gfeher wrote:
>>
>> May I also change the asserts in the other unit test in this file?
>
>> On 2010/08/20 17:58:51, Jói wrote:
>> > I'd change all the asserts should to self.assertTrue(); I'm not sure
>
> if the
>>
>> unit
>> > test output will "look right" if it's an assert rather than a
>
> self.assertTrue
>
>
>
> Done.
>
> http://codereview.chromium.org/3155033/diff/16001/17003#newcode113
> tools/grit/grit/node/misc_unittest.py:113: ['uncond1.rc', 'only_fr.adm',
> 'only_fr.plist', 'doc.html', 'uncond2.adm'])
> On 2010/08/20 17:58:51, Jói wrote:
>>
>> +80
>
> Done.
>
> http://codereview.chromium.org/3155033/show
>