Closed
Bug 781565
Opened 12 years ago
Closed 12 years ago
ffprofile_{unix,win32}.py are misnamed and contain duplicate code
Categories
(Testing :: Talos, defect)
Testing
Talos
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: k0scist, Unassigned)
Details
(Whiteboard: [good first bug][mentor=jhammel][lang=py])
Attachments
(2 files)
8.18 KB,
patch
|
k0scist
:
review+
|
Details | Diff | Splinter Review |
8.95 KB,
patch
|
k0scist
:
review+
|
Details | Diff | Splinter Review |
http://hg.mozilla.org/build/talos/file/19fb848f813e/talos/ffprofile_win32.py http://hg.mozilla.org/build/talos/file/19fb848f813e/talos/ffprofile_unix.py So firstly this has nothing to firefox or profiles. There is only one function in each of these files, MakeDirectoryContentsWritable. There is also high duplication between these functions, only the mode is different, as well as high duplication between those in the ffprocess*.py files. This function should live in one place, either utils.py. It should determine the modeline from whether it is windows or unix. This also should probably go in mozfile: https://bugzilla.mozilla.org/show_bug.cgi?id=774916
Reporter | ||
Comment 1•12 years ago
|
||
see also https://bugzilla.mozilla.org/show_bug.cgi?id=781324
Reporter | ||
Updated•12 years ago
|
Whiteboard: [good first bug][mentor=jhammel][lang=py]
i'm looking to get involved with the mozilla community. anyone willing to help me get started on this bug, if not already assigned?
Reporter | ||
Comment 3•12 years ago
|
||
(In reply to sam from comment #2) > i'm looking to get involved with the mozilla community. anyone willing to > help me get started on this bug, if not already assigned? That would be me
as in you are already working on it, or you are the mentor and need someone to take it on?
Reporter | ||
Comment 5•12 years ago
|
||
Sorry, I should have clarified: I would be that mentor :) Feel free to stop by irc://irc.mozilla.org/#ateam and say hi
Attachment #652518 -
Flags: review?(jhammel)
Reporter | ||
Comment 7•12 years ago
|
||
Comment on attachment 652518 [details] [diff] [review] any changes required ? This looks good! + os_name=os.name + if os_name=='posix': + mod=0755 + elif os_name=='nt': + mod=0777 + else: + print('WARNING : this action is not supported on your current os') It might be better to use mozinfo here, but probably not necessary. (maybe?)
Attachment #652518 -
Flags: review?(jhammel) → review+
Reporter | ||
Comment 8•12 years ago
|
||
pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=0e385d38a11d
Reporter | ||
Comment 9•12 years ago
|
||
This doesn't quite work yet: Traceback (most recent call last): File "run_tests.py", line 298, in <module> main() File "run_tests.py", line 295, in main run_tests(parser) File "run_tests.py", line 250, in run_tests talos_results.add(mytest.runTest(browser_config, test)) File "/home/cltbld/talos-slave/talos-data/talos/ttest.py", line 262, in runTest profile_dir, temp_dir = self.createProfile(test_config['profile_path'], browser_config) File "/home/cltbld/talos-slave/talos-data/talos/ttest.py", line 132, in createProfile browser_config['webserver']) File "/home/cltbld/talos-slave/talos-data/talos/ffsetup.py", line 217, in CreateTempProfileDir self._hostproc.MakeDirectoryContentsWritable(profile_dir) AttributeError: 'LinuxProcess' object has no attribute 'MakeDirectoryContentsWritable' program finished with exit code 1 elapsedTime=0.748504
Comment 10•12 years ago
|
||
Try run for 0e385d38a11d is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=0e385d38a11d Results (out of 55 total builds): exception: 7 success: 7 failure: 41 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jhammel@mozilla.com-0e385d38a11d
Comment 11•12 years ago
|
||
Attachment #653080 -
Flags: review?(jhammel)
Reporter | ||
Updated•12 years ago
|
Attachment #653080 -
Flags: review?(jhammel) → review+
Reporter | ||
Comment 12•12 years ago
|
||
(In reply to jdev005 from comment #11) > Created attachment 653080 [details] [diff] [review] > this should do it :) pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=b9da67ca43ad
Comment 13•12 years ago
|
||
Try run for b9da67ca43ad is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=b9da67ca43ad Results (out of 69 total builds): success: 62 failure: 7 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jhammel@mozilla.com-b9da67ca43ad
Reporter | ||
Comment 14•12 years ago
|
||
(In reply to Mozilla RelEng Bot from comment #13) > Try run for b9da67ca43ad is complete. > Detailed breakdown of the results available here: > https://tbpl.mozilla.org/?tree=Try&rev=b9da67ca43ad > Results (out of 69 total builds): > success: 62 > failure: 7 > Builds (or logs if builds failed) available at: > http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jhammel@mozilla. > com-b9da67ca43ad Looks very green! :)
Reporter | ||
Comment 15•12 years ago
|
||
pushed: http://hg.mozilla.org/build/talos/rev/c986567c686a Thanks for the patch! I love to see useless code disappear
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•