Cause mini_installer tests to noop on component builds.

Cause mini_installer tests to noop on component builds.

BUG=377839, 264859
TEST=/src/chrome/test/mini_installer/test_installer.py

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=273429

[grt (UTC plus 2), 2014-05-28 16:04:08]

the buildbot tooling knows whether it's running a component build or not and
could be made to skip the test for component builds. this wouldn't require any
code changes in the mini_installer or the test. what do you think about
suppressing the test at that level instead of by adding special code for a
conversation between the test and the mini_installer?

[robertshield, 2014-05-28 18:11:08]

On 2014/05/28 16:04:08, grt wrote:
> the buildbot tooling knows whether it's running a component build or not and
> could be made to skip the test for component builds. this wouldn't require any
> code changes in the mini_installer or the test. what do you think about
> suppressing the test at that level instead of by adding special code for a
> conversation between the test and the mini_installer?

I don't like having logic in the bot configs. Our build/test infrastructure is
obscure, even knowing where to look to inspect how it works is non-obvious. 

I'd rather, as a general policy, keep all logic possible out of bot scripts
and in the code itself since I feel that makes it much easier for developers to
themselves test what is running on the bots.

In this case, this policy incurs the cost of (a very) little bit of extra
shipping
code, but I feel this cost is worth the benefits.

What say you?

[grt (UTC plus 2), 2014-05-28 18:51:43]

okay. lgtm w/ a comment nit

https://codereview.chromium.org/304673004/diff/20001/chrome/installer/mini_in...
File chrome/installer/mini_installer/configuration.h (right):

https://codereview.chromium.org/304673004/diff/20001/chrome/installer/mini_in...
chrome/installer/mini_installer/configuration.h:59: // This will cause
mini_installer to exit and return 1 if this is
my brain is about to 'splode. 0 exit code means success, so the query should
return 0 for a component install and non-zero for false. but this looks backward
when you look at the python. what to do? personally, i would have gone the other
way, but i can see the logic in doing it this way. oh well.

https://codereview.chromium.org/304673004/diff/20001/chrome/installer/mini_in...
File chrome/installer/mini_installer/mini_installer.cc (right):

https://codereview.chromium.org/304673004/diff/20001/chrome/installer/mini_in...
chrome/installer/mini_installer/mini_installer.cc:781: // Return whether this is
a component build then exit. This is used by
nit: returning is exiting. how about
    // Exit immediately with 1 or 0 to indicate component build or not.
or something like that

[robertshield, 2014-05-28 21:27:29]

Thanks!

https://codereview.chromium.org/304673004/diff/20001/chrome/installer/mini_in...
File chrome/installer/mini_installer/configuration.h (right):

https://codereview.chromium.org/304673004/diff/20001/chrome/installer/mini_in...
chrome/installer/mini_installer/configuration.h:59: // This will cause
mini_installer to exit and return 1 if this is
On 2014/05/28 18:51:44, grt wrote:
> my brain is about to 'splode. 0 exit code means success, so the query should
> return 0 for a component install and non-zero for false. but this looks
backward
> when you look at the python. what to do? personally, i would have gone the
other
> way, but i can see the logic in doing it this way. oh well.

Yeah, I chewed on that for a bit too, but 1 = yes, 0 = no seemed (just
marginally) more reasonable.

https://codereview.chromium.org/304673004/diff/20001/chrome/installer/mini_in...
File chrome/installer/mini_installer/mini_installer.cc (right):

https://codereview.chromium.org/304673004/diff/20001/chrome/installer/mini_in...
chrome/installer/mini_installer/mini_installer.cc:781: // Return whether this is
a component build then exit. This is used by
On 2014/05/28 18:51:44, grt wrote:
> nit: returning is exiting. how about
>     // Exit immediately with 1 or 0 to indicate component build or not.
> or something like that

Done.

[robertshield, 2014-05-28 21:34:16]

The CQ bit was checked by robertshield@chromium.org

[commit-bot: I haz the power, 2014-05-28 21:34:59]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/robertshield@chromium.org/304673004/40001

[commit-bot: I haz the power, 2014-05-29 01:15:44]

Change committed as 273429

[gab, 2014-06-06 16:08:11]

https://codereview.chromium.org/304673004/diff/40001/chrome/test/mini_install...
File chrome/test/mini_installer/test_installer.py (right):

https://codereview.chromium.org/304673004/diff/40001/chrome/test/mini_install...
chrome/test/mini_installer/test_installer.py:285: return 0
Since we return here why do we also need logic via an environment variable to
now show UI in component mini_installer.exe's? Seems mini_installer.exe will
never be ran in that mode (and even if it was, it's wrong to continue).

[robertshield, 2014-06-06 17:19:21]

https://codereview.chromium.org/304673004/diff/40001/chrome/test/mini_install...
File chrome/test/mini_installer/test_installer.py (right):

https://codereview.chromium.org/304673004/diff/40001/chrome/test/mini_install...
chrome/test/mini_installer/test_installer.py:285: return 0
On 2014/06/06 16:08:11, gab wrote:
> Since we return here why do we also need logic via an environment variable to
> now show UI in component mini_installer.exe's? Seems mini_installer.exe will
> never be ran in that mode (and even if it was, it's wrong to continue).

IsComponentBuild calls mini_installer.

[gab, 2014-06-06 17:25:03]

https://codereview.chromium.org/304673004/diff/40001/chrome/test/mini_install...
File chrome/test/mini_installer/test_installer.py (right):

https://codereview.chromium.org/304673004/diff/40001/chrome/test/mini_install...
chrome/test/mini_installer/test_installer.py:285: return 0
On 2014/06/06 17:19:21, robertshield wrote:
> On 2014/06/06 16:08:11, gab wrote:
> > Since we return here why do we also need logic via an environment variable
to
> > now show UI in component mini_installer.exe's? Seems mini_installer.exe will
> > never be ran in that mode (and even if it was, it's wrong to continue).
> 
> IsComponentBuild calls mini_installer.
> 

I see, but couldn't the early return for the query-component-build be done
before the UI prompt? Seems weird to let the UI prompt fall through only to
reach another early return...

[robertshield, 2014-06-06 17:49:06]

https://codereview.chromium.org/304673004/diff/40001/chrome/test/mini_install...
File chrome/test/mini_installer/test_installer.py (right):

https://codereview.chromium.org/304673004/diff/40001/chrome/test/mini_install...
chrome/test/mini_installer/test_installer.py:285: return 0
On 2014/06/06 17:25:03, gab wrote:
> On 2014/06/06 17:19:21, robertshield wrote:
> > On 2014/06/06 16:08:11, gab wrote:
> > > Since we return here why do we also need logic via an environment variable
> to
> > > now show UI in component mini_installer.exe's? Seems mini_installer.exe
will
> > > never be ran in that mode (and even if it was, it's wrong to continue).
> > 
> > IsComponentBuild calls mini_installer.
> > 
> 
> I see, but couldn't the early return for the query-component-build be done
> before the UI prompt? Seems weird to let the UI prompt fall through only to
> reach another early return...

Sure, that would make mini_installer.cc a bit messier imo, but it's roughly
equivalent and this is temporary anyway, so patches are welcome :-)

[gab, 2014-06-10 18:43:43]

https://codereview.chromium.org/304673004/diff/40001/chrome/test/mini_install...
File chrome/test/mini_installer/test_installer.py (right):

https://codereview.chromium.org/304673004/diff/40001/chrome/test/mini_install...
chrome/test/mini_installer/test_installer.py:285: return 0
On 2014/06/06 17:49:06, robertshield wrote:
> On 2014/06/06 17:25:03, gab wrote:
> > On 2014/06/06 17:19:21, robertshield wrote:
> > > On 2014/06/06 16:08:11, gab wrote:
> > > > Since we return here why do we also need logic via an environment
variable
> > to
> > > > now show UI in component mini_installer.exe's? Seems mini_installer.exe
> will
> > > > never be ran in that mode (and even if it was, it's wrong to continue).
> > > 
> > > IsComponentBuild calls mini_installer.
> > > 
> > 
> > I see, but couldn't the early return for the query-component-build be done
> > before the UI prompt? Seems weird to let the UI prompt fall through only to
> > reach another early return...
> 
> Sure, that would make mini_installer.cc a bit messier imo, but it's roughly
> equivalent and this is temporary anyway, so patches are welcome :-)

"temporary" ;-).

Are the mini_installer_tests back on the try bots?