Closed Bug 1716996 Opened 3 years ago Closed 3 years ago

Replace waitForCondition use in customizableui tests with TestUtils.waitForCondition

Categories

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

task

Tracking

()

RESOLVED FIXED
92 Branch
Tracking Status
firefox92 --- fixed

People

(Reporter: standard8, Assigned: hardikshah.hs2015, Mentored)

References

(Blocks 1 open bug)

Details

(Keywords: good-first-bug, Whiteboard: [lang=js])

Attachments

(2 files, 1 obsolete file)

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.

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

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

Summary: Replace waitForCondition use in preferences tests with TestUtils.waitForCondition → Replace waitForCondition use in customizableui tests with TestUtils.waitForCondition
Severity: -- → S3
Priority: -- → P3

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

Flags: needinfo?(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.

Flags: needinfo?(standard8)

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

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

Flags: needinfo?(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.

Flags: needinfo?(standard8)

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

<<<

Flags: needinfo?(standard8)

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

./mach sorry ;)

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.

Flags: needinfo?(standard8)

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

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

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.

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

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

Attached image error.png (deleted) —

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

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

Assignee: nobody → hardikshah.hs2015
Status: NEW → ASSIGNED

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

Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/686a4e778a0f
Replace waitForCondition use in customizableui tests with TestUtils.waitForCondition r=Standard8
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 92 Branch
Attachment #9230431 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: