1530785 - Replace waitForCondition in browser_913972_currentset_overflow.js withTestUtils.waitForCondition

Status: RESOLVED FIXED

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

Description

[Johann Hofmann [:johannh]]

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.

Comment 1

[asish7295]

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

Comment 2

[sunitha98selvan]

I would like to work on this bug.

Comment 3

[Johann Hofmann [:johannh]]

asish7295 asked first :)

Comment 4

[asish7295]

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?

Comment 5

[ekikoh]

(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.

Comment 6

[asish7295]

(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

Comment 7

[Johann Hofmann [:johannh]]

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.

Comment 8

[asish7295]

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

Comment 9

[asish7295]

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

Comment 10

[ivan.topolcic]

Attachment: Proposed patch to remove deprecated calls to waitForCondition

Comment 11

[ivan.topolcic]

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.

Comment 12

[asish7295]

(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

Comment 13

[afshan.shokath]

Hello asish7295,
Are you still working on this?

Comment 14

[Johann Hofmann [:johannh]]

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

Comment 15

[afshan.shokath]

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

Comment 16

[Johann Hofmann [:johannh]]

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

Comment 17

[Michael Krasnov]

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

Comment 18

[Johann Hofmann [:johannh]]

Absolutely, thank you!

Comment 19

[Michael Krasnov]

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!

Comment 20

[Johann Hofmann [:johannh]]

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 :)

Comment 21

[Michael Krasnov]

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!

Comment 22

[Michael Krasnov]

(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 23

[Michael Krasnov]

Attachment: Proposed patch to change calls to deprecated waitForCondition

Comment 24

[Michael Krasnov]

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

Proposed patch to switch from deprecated calls to waitForCondition

Comment 25

[Johann Hofmann [:johannh]]

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 :)

Comment 26

[Michael Krasnov]

Attachment: Bug 1530785 - Change calls to deprecated waitForCondition

Comment 27

[Michael Krasnov]

(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!

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/85c9d87c3079
Change calls to deprecated waitForCondition r=johannh

Comment 29

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/85c9d87c3079