Closed
Bug 1517059
Opened 3 years ago
Closed 3 years ago
Tidy up Mozmill/JSBridge resources
Categories
(Thunderbird :: Testing Infrastructure, enhancement)
Thunderbird
Testing Infrastructure
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 66.0
People
(Reporter: darktrojan, Assigned: darktrojan)
Details
Attachments
(1 file, 1 obsolete file)
92.13 KB,
patch
|
aceman
:
review+
|
Details | Diff | Splinter Review |
In this bug I'm going to lint all the Python in mail/test/mozmill and mail/test/resources, and remove the docs, scripts, and test directories which are unused and seriously out-of-date.
Assignee | ||
Comment 1•3 years ago
|
||
I know you love linting stuff aceman – how about some Python for a change? Buried in this patch, I've moved the profile directory used for mozmill tests out of the objdir and into the temporary directory (whatever tempfile.gettempdir() returns). This has an advantage in that by setting $TEMP you can move the profile dir, to /dev/shm for example, which avoids writing to disk at all.
Attachment #9033827 -
Flags: review?(acelists)
Assignee | ||
Comment 2•3 years ago
|
||
I'm not going to post a patch that only deletes 76 files, because that's ridiculous. So this ni? is really an r? on that operation. The files are: mail/test/resources/mozmill/docs/* mail/test/resources/mozmill/scripts/* mail/test/resources/mozmill/test/*
Flags: needinfo?(philipp)
Comment 3•3 years ago
|
||
Luckly this didn't get buried in my ni queue :D I think it would be best for Jörg to decide on this, I believe he has been more involved with mozmill than me. From a brief look I didn't see anything of value.
Flags: needinfo?(philipp) → needinfo?(jorgk)
Comment 4•3 years ago
|
||
I guess it's OK as long as you don't delete mail/test/resources/mozmill/scripts/mozmill ;-) - Aceman is reviewing here, so we can confirm.
Flags: needinfo?(jorgk) → needinfo?(acelists)
Assignee | ||
Comment 5•3 years ago
|
||
That's not a real file.
Comment 6•3 years ago
|
||
Grrr, what happened there, cut&paste error :-( - I meant: mail/test/resources/mozmill/mozmill ;-)
(In reply to Geoff Lankow (:darktrojan) from comment #1) > Created attachment 9033827 [details] [diff] [review] > 1517059-mozmill-python-1.diff > > I know you love linting stuff aceman – how about some Python for a change? How did you create the patch? Is there a tool for python linting? Or is it manually decided cleanup? > Buried in this patch, I've moved the profile directory used for mozmill > tests out of the objdir and into the temporary directory (whatever > tempfile.gettempdir() returns). That sounds useful, I think xpcshell is already in temp dir so mozmill can follow. What does tempfile.gettempdir() return? Is it e.g. /tmp on Linux, or does it append (create) some random subdirectory (thinking of mktemp() behaviour)? > This has an advantage in that by setting > $TEMP you can move the profile dir, to /dev/shm for example, which avoids > writing to disk at all. Yes, nice.
Flags: needinfo?(acelists)
Comment on attachment 9033827 [details] [diff] [review] 1517059-mozmill-python-1.diff Review of attachment 9033827 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/test/mozmill/runtest.py @@ +188,5 @@ > pass > else: > self.set_preferences(self.menubar_preferences) > > + if (wrapper is not None and hasattr(wrapper, "NO_ACCOUNTS") and wrapper.NO_ACCOUNTS): Why this change? Do we want long lines in python? @@ +533,5 @@ > if not parent_dir: > return > > from base64 import b64decode > from os import fdopen 'os' is also already imported above.
Attachment #9033827 -
Flags: feedback+
Updated•3 years ago
|
Keywords: leave-open
Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/0883f1e35c11 Tidy up Mozmill/JSBridge resources: remove mail/test/resources/mozmill/scripts/. r=darktrojan
Comment 10•3 years ago
|
||
Sorry, no patch to land, so removed some of the stuff mentioned in comment #2. I was going to kill it all, but I asked myself whether docs had any value.
Assignee | ||
Comment 11•3 years ago
|
||
(In reply to :aceman from comment #7) > How did you create the patch? Is there a tool for python linting? Or is it > manually decided cleanup? There's a standard (flake8) which m-c code is checked for automatically but not c-c code. I've used a plugin for my editor which might be slightly different but close enough. > > Buried in this patch, I've moved the profile directory used for mozmill > > tests out of the objdir and into the temporary directory (whatever > > tempfile.gettempdir() returns). > > That sounds useful, I think xpcshell is already in temp dir so mozmill can > follow. > What does tempfile.gettempdir() return? Is it e.g. /tmp on Linux, or does it > append (create) some random subdirectory (thinking of mktemp() behaviour)? It's just /tmp, so our profile directory becomes /tmp/mozmillprofile. xpcshell does this already (unless you run more than one test, annoyingly) and mochitest does this already. (In reply to :aceman from comment #8) > > + if (wrapper is not None and hasattr(wrapper, "NO_ACCOUNTS") and wrapper.NO_ACCOUNTS): > > Why this change? Do we want long lines in python? I'll fix it and the other line longer than 100 chars, just for you. :) > > from base64 import b64decode > > from os import fdopen > > 'os' is also already imported above. Well spotted. These imports shouldn't be here but someone got lazy (me). Moving to the top.
Assignee | ||
Comment 12•3 years ago
|
||
Attachment #9034043 -
Flags: review?(acelists)
Comment 13•3 years ago
|
||
Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/246f3ec47ccd Tidy up Mozmill/JSBridge resources: remove mail/test/resources/mozmill/test/. r=darktrojan
Assignee | ||
Comment 14•3 years ago
|
||
(In reply to Geoff Lankow (:darktrojan) from comment #11) > (In reply to :aceman from comment #7) > > How did you create the patch? Is there a tool for python linting? Or is it > > manually decided cleanup? > > There's a standard (flake8) which m-c code is checked for automatically but > not c-c code. I've used a plugin for my editor which might be slightly > different but close enough. I just got a flake8 error on a Try build. So apparently it IS checked … sometimes … ?
Assignee | ||
Updated•3 years ago
|
Attachment #9033827 -
Attachment is obsolete: true
Attachment #9033827 -
Flags: review?(acelists)
![]() |
||
Comment 15•3 years ago
|
||
Comment on attachment 9034043 [details] [diff] [review] 1517059-mozmill-python-2.diff Review of attachment 9034043 [details] [diff] [review]: ----------------------------------------------------------------- Thanks.
Attachment #9034043 -
Flags: review?(acelists) → review+
Comment 16•3 years ago
|
||
Pushed by geoff@darktrojan.net: https://hg.mozilla.org/comm-central/rev/1ae2793b99f0 Tidy up Mozmill/JSBridge resources: remove mail/test/resources/mozmill/docs/. r=me
Assignee | ||
Updated•3 years ago
|
Keywords: leave-open
Comment 17•3 years ago
|
||
Pushed by geoff@darktrojan.net: https://hg.mozilla.org/comm-central/rev/f2f995d7cf9a Tidy up Mozmill/JSBridge resources; r=aceman
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•3 years ago
|
Target Milestone: --- → Thunderbird 66.0
You need to log in
before you can comment on or make changes to this bug.
Description
•