1716996 - Replace waitForCondition use in customizableui tests with TestUtils.waitForCondition

Status: RESOLVED FIXED

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

Description

[Mark Banner (:standard8)]

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

waitForCondition in the browser/components/customizableui test directory can be replaced by the TestUtils.waitForCondition utility function.

The code in question is here: https://searchfox.org/mozilla-central/search?q=+waitForCondition&path=customizableui&case=false&regexp=false

Once replaced, the function definition in the head.js file can be removed. In addition to removing the function, the import for Promise should also be able to be removed.

For instructions on how to get your local build of Firefox up and running and submit your patch, see https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html

You can run the tests with the ./mach mochitest command:

./mach mochitest browser/components/customizableui

This will run all the tests in the directory, to make sure that the head.js changes don't adversely affect anything else.

You should also make sure that ESLint passed:

./mach eslint browser/components/customizableui

Note this bug will be auto-assigned when the first patch is attached.

Comment 1

[hardikshah.hs2015]

I want to work on this issue as my first contribution, should I take up this? @Mark Banner

Comment 2

[Mark Banner (:standard8)]

(In reply to hardikshah.hs2015 from comment #1)

I want to work on this issue as my first contribution, should I take up this? @Mark Banner

Yes, that's fine. That's what good first bugs are for :)

Comment 3

[Putta Bhanu Prakash]

Hy @Mark, Is the issue still open for contribution? I want to get started with Mozilla Open Source with this issue.

Comment 4

[Mark Banner (:standard8)]

(In reply to Putta Bhanu Prakash from comment #3)

Hy @Mark, Is the issue still open for contribution? I want to get started with Mozilla Open Source with this issue.

Sure, no-one else has submitted a patch yet and the last comment was a couple of weeks ago.

Comment 5

[Putta Bhanu Prakash]

Thanks, Mark. I will get started on this issue tomorrow.

Comment 6

[Putta Bhanu Prakash]

Hello Mark, How can I clone this repo on my local machine ?

Comment 7

[Mark Banner (:standard8)]

Sorry, I put the wrong link in my original comment, you want to follow: https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html

Ask in #introduction on Matrix if you get issues getting things set up.

Comment 8

[Putta Bhanu Prakash]

Hy @Mark, I was trying to build the system but keep running to this error. This is the error log I received, please suggest what I can do to resolve it.

0:00.32 Clobber not needed.
0:00.35 Adding make options from /home/bprakashputta/code/opensource/mozilla-central/mozconfig
MOZ_OBJDIR=/home/bprakashputta/code/opensource/mozilla-central/obj-x86_64-pc-linux-gnu
OBJDIR=/home/bprakashputta/code/opensource/mozilla-central/obj-x86_64-pc-linux-gnu
FOUND_MOZCONFIG=/home/bprakashputta/code/opensource/mozilla-central/mozconfig
export FOUND_MOZCONFIG
0:00.35 /usr/bin/make -f client.mk -s
1:19.60 abort: error: TLS/SSL connection has been closed (EOF) (_ssl.c:727)
1:19.64 Error running mach:
1:19.64 ['--log-no-times', 'artifact', 'install']
1:19.64 The error occurred in code that was called by the mach command. This is either
1:19.64 a bug in the called code itself or in the way that mach is calling it.
1:19.64 You can invoke |./mach busted| to check if this issue is already on file. If it
1:19.64 isn't, please use |./mach busted file artifact| to report it. If |./mach busted| is
1:19.64 misbehaving, you can also inspect the dependencies of bug 1543241.
1:19.64 If filing a bug, please include the full output of mach, including this error
1:19.64 message.
1:19.64 The details of the failure are as follows:
1:19.64 requests.exceptions.SSLError: HTTPSConnectionPool(host='hg.mozilla.org', port=443): Max retries exceeded with url: /releases/mozilla-release/json-pushes?changeset=fac132d8859f8970bd2b754cf923255e87f3c4f6&version=2&tipsonly=1 (Caused by SSLError(SSLZeroReturnError(6, 'TLS/SSL connection has been closed (EOF) (_ssl.c:1131)')))
1:19.64 File "/home/bprakashputta/code/opensource/mozilla-central/python/mozbuild/mozbuild/artifact_commands.py", line 204, in artifact_install
1:19.64 return artifacts.install_from(source, distdir or self.distdir)
1:19.64 File "/home/bprakashputta/code/opensource/mozilla-central/python/mozbuild/mozbuild/artifacts.py", line 1525, in install_from
1:19.64 return self.install_from_recent(distdir)
1:19.64 File "/home/bprakashputta/code/opensource/mozilla-central/python/mozbuild/mozbuild/artifacts.py", line 1430, in install_from_recent
1:19.64 return self._install_from_hg_pushheads(hg_pushheads, distdir)
1:19.64 File "/home/bprakashputta/code/opensource/mozilla-central/python/mozbuild/mozbuild/artifacts.py", line 1402, in _install_from_hg_pushheads
1:19.65 for trees, hg_hash in hg_pushheads:
1:19.65 File "/home/bprakashputta/code/opensource/mozilla-central/python/mozbuild/mozbuild/artifacts.py", line 1251, in _find_pushheads
1:19.65 candidate_pushheads = self._pushheads_from_rev(
1:19.65 File "/home/bprakashputta/code/opensource/mozilla-central/python/mozbuild/mozbuild/artifacts.py", line 1098, in _pushheads_from_rev
1:19.65 pushid = pushhead_cache.parent_pushhead_id(tree, rev)
1:19.65 File "/home/bprakashputta/code/opensource/mozilla-central/python/mozbuild/mozbuild/artifacts.py", line 786, in wrapper
1:19.65 result = method(self, *args, **kwargs)
1:19.65 File "/home/bprakashputta/code/opensource/mozilla-central/python/mozbuild/mozbuild/artifacts.py", line 896, in parent_pushhead_id
1:19.65 req = requests.get(
1:19.65 File "/home/bprakashputta/code/opensource/mozilla-central/third_party/python/requests/requests/api.py", line 76, in get
1:19.65 return request('get', url, params=params, **kwargs)
1:19.65 File "/home/bprakashputta/code/opensource/mozilla-central/third_party/python/requests/requests/api.py", line 61, in request
1:19.65 return session.request(method=method, url=url, **kwargs)
1:19.65 File "/home/bprakashputta/code/opensource/mozilla-central/third_party/python/requests/requests/sessions.py", line 542, in request
1:19.65 resp = self.send(prep, **send_kwargs)
1:19.65 File "/home/bprakashputta/code/opensource/mozilla-central/third_party/python/requests/requests/sessions.py", line 655, in send
1:19.65 r = adapter.send(request, **kwargs)
1:19.65 File "/home/bprakashputta/code/opensource/mozilla-central/third_party/python/requests/requests/adapters.py", line 514, in send
1:19.65 raise SSLError(e, request=request)
1:19.71 make[3]: *** [Makefile:126: recurse_artifact] Error 1
1:19.71 make[2]: *** [/home/bprakashputta/code/opensource/mozilla-central/config/recurse.mk:34: artifact] Error 2
1:19.71 make[1]: *** [/home/bprakashputta/code/opensource/mozilla-central/config/rules.mk:355: default] Error 2
1:19.71 make: *** [client.mk:65: build] Error 2
1:19.71 0 compiler warnings present.
Glean could not be found, so telemetry will not be reported. You may need to run |mach bootstrap|.

<<<

Comment 9

[maaike.dumont]

Were you able to run the /.mach build and run commands correctly?

Comment 10

[maaike.dumont]

./mach sorry ;)

Comment 11

[Mark Banner (:standard8)]

Hi Putta, I've not seen that error before. My only guess would be to check your connection and try running the command a second time.

If that doesn't work, please could you try asking on #introduction on Matrix as there are others who might have run into the same issue or will probably have a better idea of the issue.

Comment 12

[Putta Bhanu Prakash]

(In reply to maaike.dumont from comment #9)

Were you able to run the /.mach build and run commands correctly?

I got the output while running ./mach build only; I still haven't run the run command as it's not built successfully.

Comment 13

[Putta Bhanu Prakash]

(In reply to Mark Banner (:standard8) from comment #11)

Hi Putta, I've not seen that error before. My only guess would be to check your connection and try running the command a second time.

If that doesn't work, please could you try asking on #introduction on Matrix as there are others who might have run into the same issue or will probably have a better idea of the issue.

Yeah, sure. I Will do that, but right now, stuck with my relatives here for a function. I will update you tomorrow.

Comment 14

[hardikshah.hs2015]

Hi Mark, as you have written

Once replaced, the function definition in the head.js file can be removed.

Can you explain it a little, please? Whether I have to remove the function completely or just replace the name.

TestUtils.waitForCondition(aConditionFn, aMaxTries = 50, aCheckInterval = 100)
{
function tryNow() {
tries++;
if (aConditionFn()) {
deferred.resolve();
} else if (tries < aMaxTries) {
tryAgain();
} else {
deferred.reject("Condition timed out: " + aConditionFn.toSource());
}
}
function tryAgain() {
setTimeout(tryNow, aCheckInterval);
}
let deferred = Promise.defer();
let tries = 0;
tryAgain();
return deferred.promise;
}

I have this currently. thanks.

Comment 15

[Mark Banner (:standard8)]

(In reply to hardikshah.hs2015 from comment #14)

Hi Mark, as you have written

Once replaced, the function definition in the head.js file can be removed.

Can you explain it a little, please? Whether I have to remove the function completely or just replace the name.

The whole function can be removed.

Comment 16

[Putta Bhanu Prakash]

(In reply to hardikshah.hs2015 from comment #14)

Hi Mark, as you have written

Once replaced, the function definition in the head.js file can be removed.

Can you explain it a little, please? Whether I have to remove the function completely or just replace the name.

TestUtils.waitForCondition(aConditionFn, aMaxTries = 50, aCheckInterval = 100)
{
function tryNow() {
tries++;
if (aConditionFn()) {
deferred.resolve();
} else if (tries < aMaxTries) {
tryAgain();
} else {
deferred.reject("Condition timed out: " + aConditionFn.toSource());
}
}
function tryAgain() {
setTimeout(tryNow, aCheckInterval);
}
let deferred = Promise.defer();
let tries = 0;
tryAgain();
return deferred.promise;
}

I have this currently. thanks.

I think you have worked on most of the bug, I will search for another bug and work on it.

Comment 17

[hardikshah.hs2015]

Attachment: error.png

Hey Mark, All the tests are passed except this one, which I haven't changed, Shall I ignore this?

Comment 18

[Mark Banner (:standard8)]

(In reply to hardikshah.hs2015 from comment #17)

Hey Mark, All the tests are passed except this one, which I haven't changed, Shall I ignore this?

I think that is ok to ignore. You can always try stashing your changes, and run it without and see if you get the same failure - either way, feel free to post the patch.

(In reply to Putta Bhanu Prakash from comment #16)

I think you have worked on most of the bug, I will search for another bug and work on it.

Sorry about that, feel free to ping me direct or ask on #introduction if you struggle finding a different bug.

Comment 19

[hardikshah.hs2015]

Attachment: Bug 1716996 - Replace waitForCondition use in customizableui tests with TestUtils.waitForCondition r=standard8

Comment 20

[hardikshah.hs2015]

Here is the patch for this bug Mark, https://phabricator.services.mozilla.com/D119490

Comment 21

[hardikshah.hs2015]

Attachment: Bug 1716996 - Replace waitForCondition use in customizableui tests with TestUtils.waitForCondition r=standard8

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/686a4e778a0f
Replace waitForCondition use in customizableui tests with TestUtils.waitForCondition r=Standard8

Comment 23

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/686a4e778a0f