Tests for the alert message scroll pane

[Natalie-the-technician]

In #2833 you said, about testing the three properties of the scroll pane
that 16b5f21 put around the message of an alert:

Not sure that focus-traversability is worth testing, but otherwise, why not

So here are the other two. Focus traversability is deliberately left out,
as you asked.

This branch adds test code only. UIFacadeImpl.java is untouched.

The height cap (AlertContentHeightTest)

• A message longer than the cap asks for the cap and not for more.
  Measured at 1920x1080: the same pane without the cap asks for 4817, with it
  for 648, which is 0.6 * 1080.
• A message that fits keeps exactly the height it would have without the cap
  (33 both ways), which is the "short messages keep the size they have today"
  from your commit message, as an assertion.
• What does not fit stays reachable: the message is laid out in full behind a
  viewport that is at most the cap high, and scrolling to the end moves it by
  exactly the part that did not fit and brings its bottom into the viewport.

Every preferred height is asked for with the content width passed in, since
prefHeight(-1) measures unwrapped text, and only after the pane has been in
a scene once, since before applyCss() the same call answers 36 instead of
4817. The uncapped comparison number comes from a second, identically built
and identically laid out ScrollPane without the override, so that both
numbers arise the same way.

The screen the cap is taken from (AlertOwnerScreenTest)

• screenOf returns the screen a window stands on, checked for every screen
  the machine has.
• The cap follows the owner's screen rather than the primary one. This one
  needs more than one screen to say anything, so it is guarded by an
  assumption and skips on a single-screen machine.
• A window that is on no screen falls back to the primary screen. This is not
  hypothetical: a Stage that has never been shown has x, y, width and
  height of NaN, and Screen.getScreensForRectangle(NaN, NaN, 1, 1)
  returns nothing. Without the orElse in screenOf, building such an alert
  throws.

On the reflection

makeScrollable and screenOf are private, and nothing outside the alert
has any business calling them, so the tests reach them by reflection rather
than widening them: the tested code stays the shipped code. If you would
rather see them package-private with a comment, say so and I will change it.

Negative controls

Each test was watched failing against a deliberately broken UIFacadeImpl,
then the break was taken back and the green run confirmed:

• Math.min(super.computePrefHeight(width), maxHeight) -> super.computePrefHeight(width):
  3 tests fail, e.g. a long message asks for 4817.0, which is more than the cap of 648.0.
• .orElse(Screen.getPrimary()) -> .orElseThrow():
  2 tests fail with java.util.NoSuchElementException: No value present.
• screenOf(owner) -> Screen.getPrimary(): nothing fails, because the
  machine these tests were run on has one screen, where the two are the same
  function. That is what the skipped assumption above is about. On a
  multi-monitor machine that test carries the difference.

Full module run: 96 tests, 0 failures, 1 skipped (the multi-screen one).

[dbarashev]

This looks good and the tests pass on my laptop, however, they fail on Github runner because of a slight mismatch in a height:

    org.opentest4j.AssertionFailedError: the pane was not given the height it asked for, so nothing below is measured on it ==> expected: <614.4> but was: <615.0>

Maybe we can compare a range +- 1 point instead of strict comparison?

[Natalie-the-technician]

Found the cause, and it is neither a font nor a wrapping difference: it is the rounding of
a fractional cap to whole pixels.

The cap is screen.visualBounds.height * 0.6. Your runner's screen is 1280x1024, so the cap
is 614.4: .github/workflows/gradle.yml runs xvfb-run ./gradlew with no -screen argument,
and xvfb-run's built-in default is -screen 0 1280x1024x24. A Region hands a child only
whole device pixels and rounds up, so a pane that asks for 614.4 is given 615.0. That is
the 0.6 you saw. On a 1920x1080 screen the cap is 648.0, already whole, and the tests pass

• every screen height that is a multiple of 5 gives a whole cap.

I reproduced your failure verbatim under Xvfb at 1280x1024: "expected: <614.4> but was:
<615.0>". The message geometry is identical on both screens - the label asks for 4817.0 at
width 360 and is laid out 5153.0 high, in System Regular 13.0, on 1024 and on 1080 alike -
so no line was added or lost. Only the cap moved.

Two things follow.

First, a +-1 on the comparison in your error message alone would not have made CI green.
The next assertion in the same test, viewportHeight <= cap + 0.5, would have failed right
after it: 615.0 against 614.9. It never got there because the first one threw. There are
eight height comparisons to convert, not one.

Second, one pixel is the right number, and it is a bound rather than a guess. The gap is
ceil(cap * renderScale) / renderScale - cap, so it is below one device pixel by
construction. For an integer screen height at scale 1 it is one of 0, 0.4, 0.8, 0.2, 0.6
depending on the height modulo 5, at most 0.8. I ran the tests on six screen heights -
1080 (gap 0.0), 900 (0.0), 768 (0.2), 1003 (0.2), 1024 (0.6) and 1002 (0.8, the arithmetic
worst case) - and they pass on all of them.

So every height that comes out of a laid-out scene now goes through one of two helpers,
assertSameHeight and assertHeightAtMost, with a single tolerance of one pixel whose comment
carries that derivation. The exact invariants stay strict: that the capped height never
exceeds the cap, that the test message is taller than the cap, and that the message is
taller than the viewport showing it, are still asserted without any slack, so the tolerance
cannot hide a cap that fails to bite.

What the tolerance costs in sharpness, per assertion, measured at 1280x1024:

• a long message asks for no more than the cap: 614.4 against an uncapped 4817.0,
  difference 4202.6, so 1:4203
• a short message keeps its height: 33.0 kept against the 614.4 it would be stretched to,
  difference 581.4, so 1:581
• the pane was given the height it asked for: 614.4 against the uncapped 5153.0,
  difference 4538.0, so 1:4538
• the visible part stays within the cap: 615.0 against a content height of 5153.0,
  difference 4538.0, so 1:4538
• scrolling moves the message by the overflow: 4538.0 against 0 if it did not move,
  so 1:4538
• the end of the message is inside the viewport: the same 4538.0, so 1:4538
• the fallback cap is the primary screen's share: 614.4 against 0, so 1:614

The worst of those is 1:581.

I checked the tolerance in both directions on each of those assertions separately, at
1920x1080: shifting the measured height by +1.1 turns every one of them red, by +0.9 they
all stay green. And moving the cap in makeScrollable itself by a real 2.0 points fails two
tests, "expected: <648.0> but was: <646.0>" in both, while 0.5 points passes. So the
tolerance still catches a regression in the cap, and it does tolerate the rounding.

One test I could not exercise: "the cap follows the screen of the owner, not the primary
screen" needs two monitors and is skipped here. Worth knowing that its assumeTrue only
checks that a second screen exists, not that it has a different height - on two equally
tall monitors it passes whichever screen the cap came from, with or without the tolerance.
That was already the case before this change.