Replace waitForCondition in browser_913972_currentset_overflow.js withTestUtils.waitForCondition

RESOLVED FIXED in Firefox 69

Status

()

task
P5
normal
RESOLVED FIXED
6 months ago
3 months ago

People

(Reporter: johannh, Assigned: mihalKrasnov, Mentored)

Tracking

({good-first-bug})

unspecified
Firefox 69
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox69 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

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

waitForCondition usage in the browser_913972_currentset_overflow.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_913972_currentset_overflow.js#21,26

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_913972_currentset_overflow.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 am an outreachy applicant. It would be nice if you could assign it to me

I would like to work on this bug.

asish7295 asked first :)

Assignee: nobody → asishkrramuk
Status: NEW → ASSIGNED

hi, whenever i try and run the command
./mach mochitest browser/components/customizableui/test/browser_913972_currentset_overflow.js
i recieve the following error :
cat: backend.TestManifestBackend.in: No such file or directory
Build configuration changed. Regenerating backend.
Traceback (most recent call last):
File "f:/mozilla-source/mozilla-central/build/gen_test_backend.py", line 39, in <module>
sys.exit(gen_test_backend())
File "f:/mozilla-source/mozilla-central/build/gen_test_backend.py", line 35, in gen_test_backend
backend.consume(data)
File "f:\mozilla-source\mozilla-central\python\mozbuild\mozbuild\backend\base.py", line 126, in consume
for obj in objs:
File "f:\mozilla-source\mozilla-central\python\mozbuild\mozbuild\frontend\emitter.py", line 173, in emit
for out in output:
File "f:\mozilla-source\mozilla-central\python\mozbuild\mozbuild\frontend\reader.py", line 885, in read_topsrcdir
for r in self.read_mozbuild(path, self.config):
File "f:\mozilla-source\mozilla-central\python\mozbuild\mozbuild\frontend\reader.py", line 1052, in read_mozbuild
raise bre
mozbuild.frontend.reader.BuildReaderError:

FATAL ERROR PROCESSING MOZBUILD FILE

The error occurred while processing the following file:

f:/mozilla-source/mozilla-central/moz.build

The underlying problem is we referenced a path that does not exist. That path is:

f:/mozilla-source/mozilla-central/caps/moz.build

Either create the file if it needs to exist or do not reference it.

mozmake.EXE: *** [f:/mozilla-source/mozilla-central/build/rebuild-backend.mk:25: backend.TestManifestBackend] Error 1
Error running mach:

['mochitest', 'browser/components/customizableui/test/browser_913972_currentset_overflow.js']

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.

i am new to firefox development and can't wrap my head arounf it . is this error because i built the project incorrectly?

Flags: needinfo?(jhofmann)

(In reply to asish7295 from comment #1)

Hi, i am an outreachy applicant. It would be nice if you could assign it to me

Hi asish7296 are you still working on this? cause I could take it up if your are not.

(In reply to ekikoh from comment #5)

(In reply to asish7295 from comment #1)

Hi, i am an outreachy applicant. It would be nice if you could assign it to me

Hi asish7296 are you still working on this? cause I could take it up if your are not.

Yes, I'm working on it

Huh, did you run ./mach build? Did that give you any errors? Maybe try ./mach clobber && ./mach build?

You should use Artifact Build for this bug so that your compile time is faster.

Flags: needinfo?(jhofmann)

my ./mach build earlier was successful . but after running ./mach clobber , ./mach build shows the following error :

mozbuild.frontend.reader.SandboxValidationError:
1:21.48 ==============================
1:21.48 FATAL ERROR PROCESSING MOZBUILD FILE
1:21.49 ==============================
1:21.49 The error occurred while processing the following file or one of the files it includes:
1:21.52 f:/mozilla-source/mozilla-central/xpcom/threads/moz.build
1:21.52 The error occurred when validating the result of the execution. The reported error is:
1:21.53 Path specified in LOCAL_INCLUDES does not exist: /caps (resolved to f:/mozilla-source/mozilla-central/caps)
1:21.53 *** Fix above errors and then restart with
1:21.55 "./mach build"
1:21.55 mozmake.EXE: *** [client.mk:115: configure] Error 1

i was able to run the build successfully , so ignore the earlier comment

Sorry for the above reply -- I didn't realize that adding an attachment would automatically create a comment.

I've proposed a patch to fix this issue. Please see my related changes via the following URL: https://bugzilla.mozilla.org/attachment.cgi?id=9049053

I've modified the tests to include TestUtils.waitForCondition() instead of the deprecated waitForCondition() function.

Running ./mach mochitest browser/components/customizableui/test/browser_913972_currentset_overflow.js shows that all tests succeed.

(In reply to ivan.topolcic from comment #11)

Sorry for the above reply -- I didn't realize that adding an attachment would automatically create a comment.

I've proposed a patch to fix this issue. Please see my related changes via the following URL: https://bugzilla.mozilla.org/attachment.cgi?id=9049053

I've modified the tests to include TestUtils.waitForCondition() instead of the deprecated waitForCondition() function.

Running ./mach mochitest browser/components/customizableui/test/browser_913972_currentset_overflow.js shows that all tests succeed.

Hi, i am assigned to this bug and working on it. So would you please take a look at other issues you might want to work on. Thanks

Hello asish7295,
Are you still working on this?

Unassigning due to inactivity, let me know if you want to pick this up again! :)

Assignee: asishkrramuk → nobody
Status: ASSIGNED → NEW

Yup I want to pick this up. Can I be assigned this one?

Assignee: nobody → afshan.shokath
Status: NEW → ASSIGNED

Unassigning due to inactivity, let me know if you want to pick this up again.

Assignee: afshan.shokath → nobody
Status: ASSIGNED → NEW
Type: enhancement → task

If no one is working on it right now, could I be assigned to it?

Absolutely, thank you!

Assignee: nobody → mihalKrasnov
Status: NEW → ASSIGNED

I have a question: where is waitForCondition that is currently used in browser_913972_currentset_overflow.js implemented? Do I need to remove it as it is not going to be used anymore? And how to check if the API is same or different for these two functions so I can safely change the client code? Thank you!

The custom waitForCondition is implemented in a shared head.js file: https://searchfox.org/mozilla-central/rev/116bd975c30746ddefc3d20e6947d1871469354f/browser/components/customizableui/test/head.js#340

It's being used in a bunch of other test files, so you can't remove it right now. The intent behind this bug is mostly to be an introductory challenge to new Firefox developers, thus it's just about replacing usage in a single file :)

TestUtils.waitForCondition is here: https://searchfox.org/mozilla-central/rev/116bd975c30746ddefc3d20e6947d1871469354f/testing/modules/TestUtils.jsm#160

You should feel free to check the implementation, but the use case isn't super complex so I'm sure we're good as long as the test passes :)

I have beet trying to get the development environment working, and mercurial will fail when I run hg clone with this message:
adding file changes
transaction abort!
rollback completed
abort: stream ended unexpectedly (got 8298 bytes, expected 32768)

And it always fails around ~8000 bytes. Do you know what way cause it? Thank you!

(In reply to Michael Krasnov from comment #21)

I have beet trying to get the development environment working, and mercurial will fail when I run hg clone with this message:
adding file changes
transaction abort!
rollback completed
abort: stream ended unexpectedly (got 8298 bytes, expected 32768)

And it always fails around ~8000 bytes. Do you know what way cause it? Thank you!

I tried cloning the repo while connected to a VPN, now it shows:
abort: An existing connection was forcibly closed by the remote host

Comment on attachment 9065922 [details] [diff] [review]
Proposed patch to change calls to deprecated waitForCondition

Proposed patch to switch from deprecated calls to waitForCondition
Attachment #9065922 - Attachment description: bug1530785.diff → Proposed patch to change calls to deprecated waitForCondition

Hi Michael, sorry, I didn't see this earlier, you will need to submit patches through Phabricator, see https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html

Thanks :)

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

Hi Michael, sorry, I didn't see this earlier, you will need to submit patches through Phabricator, see https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html

Thanks :)

Done!

Attachment #9065922 - Attachment is obsolete: true
Attachment #9067623 - Attachment description: Bug 1530785 - Proposed patch to change calls to deprecated waitForCondition → Bug 1530785 - Patch to change calls to deprecated waitForCondition
Attachment #9067623 - Attachment description: Bug 1530785 - Patch to change calls to deprecated waitForCondition → Bug 1530785 - Proposed patch to change calls to deprecated waitForCondition
Attachment #9067623 - Flags: checkin+
Attachment #9067623 - Attachment description: Bug 1530785 - Proposed patch to change calls to deprecated waitForCondition → Bug 1530785 - Change calls to deprecated waitForCondition
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/85c9d87c3079
Change calls to deprecated waitForCondition r=johannh
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 69
You need to log in before you can comment on or make changes to this bug.