Intermittent TestDllInterceptor.exe | test failed with return code 1

RESOLVED FIXED in Firefox 55

Status

()

Core
General
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: Treeherder Bug Filer, Assigned: handyman)

Tracking

({intermittent-failure})

unspecified
mozilla55
intermittent-failure
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

09:46:18     INFO -  TEST-FAILED | WindowsDllInterceptor | Failed to execute hooked function SetCursorPos from user32.dll

Perhaps the cursor is changing between GetCursorPos and SetCursorPos?

I wonder if it would make more sense to just give SetCursorPos garbage and assert that it fails.
Flags: needinfo?(davidp99)
(Assignee)

Comment 2

a year ago
Created attachment 8872779 [details] [diff] [review]
ignoreReturn.patch

> Perhaps the cursor is changing between GetCursorPos and SetCursorPos?
> 
> I wonder if it would make more sense to just give SetCursorPos garbage and
> assert that it fails.

Actually, its the SetCursorPos call that is failing -- the test shouldn't care if the cursor changes positions between calls.  Indeed, docs don't mention how SetCursorPos _can_ fail (it takes 2 ints as position and clips them if they are out of bounds so all inputs are valid).  Usually, this means a security failure.  Rather than care what the answer is, I've removed the return value check.  The tests are really intended to be a sanity check that we didn't write garbage assembler into the harness.  Their failures are almost certainly going to be crashes.  (You'll see other methods in there where I don't check the return value for because they were also problematic.)
Flags: needinfo?(davidp99)
Attachment #8872779 - Flags: review?(dmajor)
(Assignee)

Updated

a year ago
Assignee: nobody → davidp99
Comment on attachment 8872779 [details] [diff] [review]
ignoreReturn.patch

Oh, if all inputs are valid then perhaps we should just pass 512,512 all the time?
Attachment #8872779 - Flags: review?(dmajor) → review+
(Assignee)

Comment 4

a year ago
Created attachment 8873131 [details] [diff] [review]
Ignore return value in SetCursorPos test (r=dmajor)

I've removed the GetCursorPos call and just used 512,512 as you suggested.  I was resisting that as we've now gone full circle back to my _original_ implementation in the old bug... but the even mild additional complexity wasn't actually warranted.
Attachment #8872779 - Attachment is obsolete: true

Comment 6

a year ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a12d99ba297f
Ignore return value of SetCursorPos in TestDllInterceptor. r=dmajor
Keywords: checkin-needed

Comment 7

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a12d99ba297f
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox55: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.