Is this because you haven't installed the cros fonts?

I've run install-build-deps.sh, which is supposed to install cros fonts right?
I run it again just in case, and it says it's up-to-date, and te test failed again.

+derat@

I run install-chromeos-fonrts.py and have .fonts.conf.
I'm on a fresh machine, so I might have missed some step though.

Sorry, I don't know anything about this test or the code it's exercising.

Does your .fonts.conf match the one suggested by the script? Does it reference the per-font configs in a Chrome OS checkout as well?

I only added settings, no per-font configs. Is the per-font config part required? 

Not sure; worth a try.

We probably shouldn't have tests that depend on hardcoded font metrics.

ohh weird - I'll see if I can repro. small changes in font metrics shouldn't affect this. The test sets up a really long cancel label ("Cancel Cancel Cancel") and there are sanity checks in the test to ensure that

 - the resulting button is between 100px wide and 200px wide
 - it's wider than the button that just says "OK"

those checks are not failing.

the test then puts both buttons on the dialog and checks that the sizes match.

the failure is saying the cancel button is 140px. It looks as though.. when sizing the OK button to match, it grows an extra 3 pixels.

One guess is that the cancel button is also growing by 3px, after the early line that locks it in const

  const int cancel_button_width = client_view()->cancel_button()->width();


i.e. replacing the failing line,


  EXPECT_EQ(cancel_button_width, client_view()->ok_button()->width());

with

  EXPECT_EQ(client_view()->cancel_button()->width(), client_view()->ok_button()->width());


might make this pass. But the preferred size of the cancel button shouldn't change during the test..


I'll see if I can repro this myself.

The test passes for me locally at r492783 using `target_os = "chromeos"`

I get cancel_button_width = 169 and ok_button_only_width = 75.

If you're getting a cancel_button_width of 140 its likely we are getting different fonts. But it should be within the tolerances of the test -- something else "weird" is going on, but I don't know what.

Perhaps you can try my suggestion above? It might help narrow down the issue.

I'll try syncing my CrOS checkout to something more recent to see if anything has changed in the last month.

Ah! Actually I can repro - was compiling in the right place, but running the wrong binary. Looking..

(and confirmed my guess -- asking for client_view()->cancel_button()->width() each time rather than caching it fixes the test).

So this is a default-button-gets-a-bold-font thing. (The default implementation of DialogDelegate::GetDefaultDialogButton() changes depending which buttons are present, which changes in the test).

-> https://chromium-review.googlesource.com/66296

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a314d1ecacb5d7fff24b94b27fe75e796653cc9b

commit a314d1ecacb5d7fff24b94b27fe75e796653cc9b
Author: Trent Apted <tapted@chromium.org>
Date: Thu Sep 14 07:43:43 2017

Fix DialogClientViewTest.LinkedWidths on ChromeOS

Buttons can change width due to getting a bold font when they become the
default dialog button. Ensure the test doesn't use default dialog
buttons since it's not interesting for this test.

Bug:  764333 
Change-Id: I968068da1dbdf749664424268ab611f5ef162712
Reviewed-on: https://chromium-review.googlesource.com/662963
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501892}
[modify] https://crrev.com/a314d1ecacb5d7fff24b94b27fe75e796653cc9b/ui/views/window/dialog_client_view_unittest.cc