Closed Bug 1530787 Opened 9 months ago Closed 9 months ago

Replace waitForCondition in browser_914138_widget_API_overflowable_toolbar.js withTestUtils.waitForCondition

Categories

(Firefox :: Toolbars and Customization, enhancement, P5)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 67
Tracking Status
firefox67 --- fixed

People

(Reporter: johannh, Assigned: jcit, Mentored)

References

Details

(Keywords: good-first-bug)

Attachments

(1 file, 2 obsolete files)

This is a good first bug for newcomers to Firefox development.

waitForCondition usage in the browser_914138_widget_API_overflowable_toolbar.js test file can be replaced by the TestUtils.waitForCondition utility function.

The code in question is here: https://searchfox.org/mozilla-central/rev/dbddac86aadf1d4871fb350bbe66db43728a9f81/browser/components/customizableui/test/browser_914138_widget_API_overflowable_toolbar.js#27,44,69,81,98

There are multiple occurrences that need to be replaced.

For instructions on how to get your local build of Firefox up and running and submit your patch, see https://developer.mozilla.org/en-US/docs/Introduction.

You can run this test with the ./mach mochitest command:

./mach mochitest browser/components/customizableui/test/browser_914138_widget_API_overflowable_toolbar.js

Please leave a comment if you would like to be assigned to this bug and feel free to ask questions here or via IRC if you're stuck.

Hi, I'd like to be assigned this as my first bug

Great, thanks!

Assignee: nobody → jcit
Status: NEW → ASSIGNED

Setting checkin-needed to get this checked in by someone...

Keywords: checkin-needed

Unable to land through Lando: it says revision blocked.

Can you take a look at this?

Flags: needinfo?(jhofmann)
Flags: needinfo?(jcit)

Looks like I messed up while creating the diff, project wasn't assigned. I'll try to submit it again

Flags: needinfo?(jcit)

Replace all calls to waitForConditon with TestUtils.waitForCondition

Attachment #9046870 - Attachment is obsolete: true
Attachment #9047057 - Attachment is obsolete: true

I'll need to look into this later. It looks like that didn't fix it. I'm gonna fix my dev environment and get this resubmitted. Sorry about all this, still learning how to contribute

Finally got it working. Was missing moz-phab.

Looks like it's not blocking in Lando anymore.

(In reply to Johann Hofmann [:johannh] from comment #4)

Setting checkin-needed to get this checked in by someone...

Removing the tag. Please add it only after you've granted review because it shows up in our landing filter.
Thanks!

Keywords: checkin-needed

Landing it myself, sorry for the confusion :)

Flags: needinfo?(jhofmann)
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c923d2b7817e
Replace waitForConditon in browser_914138 with TestUtils.waitForCondition r=johannh
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
You need to log in before you can comment on or make changes to this bug.