Remove a TODO to convert bool failed, bool failure_ignored to an enum.

Remove a TODO to convert bool failed, bool failure_ignored to an enum.

(This was a TODO for phajdan.jr).

BUG=None
TEST=trybots
TBR=phajdan.jr@chromium.org

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

[tfarina, 2011-01-11 16:18:28]

Please, take a look.

[Paweł Hajdan Jr., 2011-01-11 16:33:11]

I'm not sure if it's worth touching, maybe we should just remove the TODO.

http://codereview.chromium.org/6177006/diff/11001/chrome/test/out_of_proc_tes...
File chrome/test/out_of_proc_test_runner.cc (right):

http://codereview.chromium.org/6177006/diff/11001/chrome/test/out_of_proc_tes...
chrome/test/out_of_proc_test_runner.cc:88: enum ResultMask {
Why do we need a mask? It should be possible to just use an enum:

RESULT_SUCCESS,
RESULT_FAILURE,
RESULT_IGNORED_FAILURE.

We don't need to OR together RESULT_FAILURE and RESULT_IGNORED_FAILURE.

In fact, I think we should add |run| to the mix, resulting in

RESULT_NOT_RUN,
RESULT_SUCCESS,
RESULT_FAILURE,
RESULT_IGNORED_FAILURE

[tfarina, 2011-01-11 16:36:49]

http://codereview.chromium.org/6177006/diff/11001/chrome/test/out_of_proc_tes...
File chrome/test/out_of_proc_test_runner.cc (right):

http://codereview.chromium.org/6177006/diff/11001/chrome/test/out_of_proc_tes...
chrome/test/out_of_proc_test_runner.cc:88: enum ResultMask {
On 2011/01/11 16:33:11, Paweł Hajdan Jr. wrote:
> Why do we need a mask?
See line 419.

> It should be possible to just use an enum:
> 
Initially I went to this way, but because of the possibility of having true and
false, I had to use a mask.
 
> RESULT_SUCCESS,
> RESULT_FAILURE,
> RESULT_IGNORED_FAILURE.
> 
> We don't need to OR together RESULT_FAILURE and RESULT_IGNORED_FAILURE.
> 
I needed the OR because of the condition at line 419.

[Paweł Hajdan Jr., 2011-01-11 16:50:21]

On Tue, Jan 11, 2011 at 17:36, <tfarina@chromium.org> wrote"
>
> I needed the OR because of the condition at line 419.


We don't need to use logical OR at that line, we can treat
RESULT_IGNORED_FAILURE as the third state, not as a flag. Anyway, I think we
should just remove this TODO, the benefit seems very doubtful to me now.

[tfarina, 2011-01-11 16:54:04]

http://codereview.chromium.org/6177006/diff/11001/chrome/test/out_of_proc_tes...
File chrome/test/out_of_proc_test_runner.cc (right):

http://codereview.chromium.org/6177006/diff/11001/chrome/test/out_of_proc_tes...
chrome/test/out_of_proc_test_runner.cc:423: printer.OnTestEnd(test_info->name(),
test_case->name(), true, results,
If it was an enum, how you would write this line?

[Paweł Hajdan Jr., 2011-01-11 16:55:18]

http://codereview.chromium.org/6177006/diff/11001/chrome/test/out_of_proc_tes...
File chrome/test/out_of_proc_test_runner.cc (right):

http://codereview.chromium.org/6177006/diff/11001/chrome/test/out_of_proc_tes...
chrome/test/out_of_proc_test_runner.cc:423: printer.OnTestEnd(test_info->name(),
test_case->name(), true, results,
On 2011/01/11 16:54:04, tfarina wrote:
> If it was an enum, how you would write this line?

Exactly the same.

[tfarina, 2011-01-11 16:58:56]

On 2011/01/11 16:55:18, Paweł Hajdan Jr. wrote:
>
http://codereview.chromium.org/6177006/diff/11001/chrome/test/out_of_proc_tes...
> File chrome/test/out_of_proc_test_runner.cc (right):
> 
>
http://codereview.chromium.org/6177006/diff/11001/chrome/test/out_of_proc_tes...
> chrome/test/out_of_proc_test_runner.cc:423:
printer.OnTestEnd(test_info->name(),
> test_case->name(), true, results,
> On 2011/01/11 16:54:04, tfarina wrote:
> > If it was an enum, how you would write this line?
> 
> Exactly the same.

Exactly the same? Sorry, I'm not seeing what you mean and how this would be
written using an enum.

Anyway, it's your call, if you want to just remove the TODO, I'm OK, just let me
know. 
If we go with the bitmask, I can include the bool run into it.

[Paweł Hajdan Jr., 2011-01-12 08:06:13]

On Tue, Jan 11, 2011 at 17:58, <tfarina@chromium.org> wrote:

> Anyway, it's your call, if you want to just remove the TODO, I'm OK, just
> let me
> know.


Please just remove the TODO then, feel free to TBR the change.

[tfarina, 2011-01-13 03:58:54]

On 2011/01/12 08:06:13, Paweł Hajdan Jr. wrote:
> Please just remove the TODO then, feel free to TBR the change.

Done.

[Paweł Hajdan Jr., 2011-01-13 08:49:27]

OK, thanks