ffprofile_{unix,win32}.py are misnamed and contain duplicate code

RESOLVED FIXED

Status

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: k0scist, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=jhammel][lang=py])

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
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)

Updated

6 years ago
Whiteboard: [good first bug][mentor=jhammel][lang=py]

Comment 2

6 years ago
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

6 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

Comment 4

6 years ago
as in you are already working on it, or you are the mentor and need someone to take it on?
(Reporter)

Comment 5

6 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

Comment 6

6 years ago
Created attachment 652518 [details] [diff] [review]
any changes required ?
Attachment #652518 - Flags: review?(jhammel)
(Reporter)

Comment 7

6 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 9

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

6 years ago
Created attachment 653080 [details] [diff] [review]
this should do it :)
Attachment #653080 - Flags: review?(jhammel)
(Reporter)

Updated

6 years ago
Attachment #653080 - Flags: review?(jhammel) → review+
(Reporter)

Comment 12

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

6 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

6 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
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.