1968629 - Make browser_dbg-editor-scroll.js stable

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[Hiroyuki Ikezoe (:hiro)]

Though the test failed when I landed the change for bug 1674687 (see bug 1674687 comment 12), I can see (probably) the same error when I run the test locally without any change.

devtools/client/debugger/test/mochitest/browser_dbg-editor-scroll.js
FAIL The new first line is visible -
Stack trace:
chrome://mochikit/content/browser-test.js:test_ok:1626
chrome://mochitests/content/browser/devtools/client/debugger/test/mochitest/browser_dbg-editor-scroll.js:testIsPositionVisible:195
chrome://mochikit/content/browser-test.js:handleTask:1170
chrome://mochikit/content/browser-test.js:_runTaskBasedTest:1242
chrome://mochikit/content/browser-test.js:Tester_execTest:1383
chrome://mochikit/content/browser-test.js:nextTest/<:1159
chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/<:1058

Comment 1

[Hiroyuki Ikezoe (:hiro)]

On my linux (wayland) environment, at this line;

inYView = coords.top > y && coords.bottom < y + height;

The coords.top is 25.449996948242188, wheares y is 29.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Hey Alexandre or Hubert, I think in this test getFirstVisibleLine might return lines where it's partially visible. So I think we should ok(isScrolledPositionVisible(dbg, firstLine + 1) rather than isScrolledPositionVisible(dbg, firstLine)?

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Attachment: 0001-Bug-1968629-Check-the-next-line-of-the-first-visible.patch

Just for a reference, this is the change what I have in my mind.

Comment 4

[Alexandre Poirot [:ochameau] [PTO back August 21]]

Hi :hiro

Sorry about you facing issues with this test...
I tried very hard to make this test deterministic and reliable, but it is actually brittle.
You are proving with bug 1674687 that is can easily be impacted by platform changes.

So feel free to change the assertion to whatever state we end up with.
The important thing is to assert that the scroll is overall correct. The very precise line and column we scroll to isn't critical as it can easily change based on font rendering which changes easily based on the OS/environment.

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Thank you, Alexandre!

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1968629 - Check the next line of the first visible line in browser_dbg-editor-scroll.js. r?ochameau

It's possible that the line returned by getFirstVisibleLine may be
partially visible.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/56819b15acf4
https://hg.mozilla.org/integration/autoland/rev/0152c4e74def
Check the next line of the first visible line in browser_dbg-editor-scroll.js. r=ochameau,devtools-reviewers

Comment 8

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/0152c4e74def