Prepare a build for user testing with clean/copy profile batch files and a cleanup batch file

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Gijs, Assigned: mconley)

Tracking

(Blocks 1 bug)

Trunk
x86
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Australis:M7][User Research Build+])

Attachments

(5 attachments, 3 obsolete attachments)

Reporter

Description

6 years ago
For the Australis user research, we've been asked to prep a build and zip it up together with some batch files. This would be windows only. There should be 3 batch files / shortcuts, as I understand it.

1. Start UX with a clean profile in a temp directory.
2. Start UX with *a copy of* the user's default profile.
3. Clean up the copy and/or temp profile.

They will need to have this by approximately June 17th.
Reporter

Updated

6 years ago
Assignee: nobody → mconley
Status: NEW → ASSIGNED
Whiteboard: [Australis:M7] → [Australis:M7][User Research Build+]
Posted file WIP script (obsolete) —
Assignee

Updated

6 years ago
Attachment #760004 - Attachment is patch: false
Comment on attachment 760004 [details]
WIP script

So bwinton brought py2exe to my attention, which makes the process of cloning the default repository quite a bit simpler. Trying to do this with a DOS batch file seemed likely to be error prone. Plus, the documentation for batch is horrendous.

This is the Python script I'm proposing. I didn't go out of my way to architect it in the nicest way, and makes the following assumptions:

1) The user is on a Windows machine
2) The user has one or more Firefox profiles

I've compiled this to an executable via py2exe, and tested it on a local XP VM, and it seems to do the job alright (although, when it errors out, the terminal window flashes and then goes away - we probably want to wait on user input before closing the window).

Thoughts? Opinions? Am I on the right track here?
Attachment #760004 - Flags: feedback?(mdeboer)
Attachment #760004 - Flags: feedback?(jaws)
Attachment #760004 - Flags: feedback?(gijskruitbosch+bugs)
Attachment #760004 - Flags: feedback?(bwinton)
Reporter

Comment 3

6 years ago
Comment on attachment 760004 [details]
WIP script

Nice going, this looks cool! And so much more readable than bat... Some comments:

<snip>
>  print "Running Firefox..."
>  subprocess.call("firefox/firefox.exe -P " + newIdentifier)

I'm concerned that this might restart at which point your subprocess call will return before the new browser is fired up and... kill the profile. Do we no longer restart on updates + add-on checks and all that? Because you can bet the user profile won't be one that was using Fx 24/25... If we're not sure, we should definitely test this case.

>  print "All done!"
>
>  destroyProfile(newProfileDir)




>if __name__ == "__main__":
>  try:
>    main()
>  except Exception as e:
>    print
>    print "Uh oh, something went wrong..."
>    print
>    print(e)
>    print "******************************"
>    print
>    print traceback.format_exc()

Yeah, we probably want to wait for input here. Can we do something like readline() or whatever python has for that to force waiting for input?


Generally, are you planning to make a separate copy of this that just uses a new profile and cleans up afterwards? IIRC we were going to have both of those, and that shouldn't take much work AFAICT, this case was much harder than that case.
Attachment #760004 - Flags: feedback?(gijskruitbosch+bugs) → feedback+
Comment on attachment 760004 [details]
WIP script

Works for me, but it's been quite a while since I've done this kind of thing…  :)
Attachment #760004 - Flags: feedback?(bwinton) → feedback+
Comment on attachment 760004 [details]
WIP script

>kIniBackupPath = os.path.join(kRootPath, "profiles.ini.bak")
>kExePath = os.path.join(os.path.dirname(os.path.realpath(sys.argv[0])), "firefox", "firefox.exe")

Does this require the participant to place this script in the Firefox directory (likely) under Program Files? I'm not sure how hard that will be for our participants, but I can picture some of them getting mixed up here.

>  # And restore the backup profiles.ini
>  os.remove(kIniPath)
>  shutil.copy(kIniBackupPath, kIniPath)

Don't forget to os.remove(kIniBackupPath) here, if shutil.copy was successful.

>  print "Running Firefox..."
>  subprocess.call("firefox/firefox.exe -P " + newIdentifier)

Might want to tack on -no-remote in case the user wants to launch their other installation of Firefox while running the test.
Attachment #760004 - Flags: feedback?(jaws) → feedback+
(In reply to :Gijs Kruitbosch from comment #3)
> Comment on attachment 760004 [details]
> WIP script
> 
> Nice going, this looks cool! And so much more readable than bat... Some
> comments:
> 
> <snip>
> >  print "Running Firefox..."
> >  subprocess.call("firefox/firefox.exe -P " + newIdentifier)
> 
> I'm concerned that this might restart at which point your subprocess call
> will return before the new browser is fired up and... kill the profile. Do
> we no longer restart on updates + add-on checks and all that? Because you
> can bet the user profile won't be one that was using Fx 24/25... If we're
> not sure, we should definitely test this case.

I wasn't sure, and I'm glad you brought this up. I just tested it, and it looks like when we do the update / add-on compat check, it doesn't actually restart the process. I was rather surprised by that, but it seems to be the case.

I tested this by running release, closing, and then running the script. So I think we're OK there.

> 
> >  print "All done!"
> >
> >  destroyProfile(newProfileDir)
> 
> 
> 
> 
> >if __name__ == "__main__":
> >  try:
> >    main()
> >  except Exception as e:
> >    print
> >    print "Uh oh, something went wrong..."
> >    print
> >    print(e)
> >    print "******************************"
> >    print
> >    print traceback.format_exc()
> 
> Yeah, we probably want to wait for input here. Can we do something like
> readline() or whatever python has for that to force waiting for input?

Yep, definitely.

> 
> Generally, are you planning to make a separate copy of this that just uses a
> new profile and cleans up afterwards? IIRC we were going to have both of
> those, and that shouldn't take much work AFAICT, this case was much harder
> than that case.

I'm tinkering with the idea of having this script be able to perform both functions, and just use some command-line arguments to choose the mode. Then having two batch files handle running the executable with the right arguments.
(In reply to Jared Wein [:jaws] from comment #5)
> Comment on attachment 760004 [details]
> WIP script
> 
> >kIniBackupPath = os.path.join(kRootPath, "profiles.ini.bak")
> >kExePath = os.path.join(os.path.dirname(os.path.realpath(sys.argv[0])), "firefox", "firefox.exe")
> 
> Does this require the participant to place this script in the Firefox
> directory (likely) under Program Files? I'm not sure how hard that will be
> for our participants, but I can picture some of them getting mixed up here.
> 

Thankfully, no! They can run it from anywhere.

> >  # And restore the backup profiles.ini
> >  os.remove(kIniPath)
> >  shutil.copy(kIniBackupPath, kIniPath)
> 
> Don't forget to os.remove(kIniBackupPath) here, if shutil.copy was
> successful.
> 

Ah, good call.

> >  print "Running Firefox..."
> >  subprocess.call("firefox/firefox.exe -P " + newIdentifier)
> 
> Might want to tack on -no-remote in case the user wants to launch their
> other installation of Firefox while running the test.

Good idea.
Comment on attachment 760004 [details]
WIP script

Ok, thanks for the feedback all. I am now confident that this is a reasonable way forward. :)
Attachment #760004 - Flags: feedback?(mdeboer)
Posted patch makeProfile.py script (obsolete) — Splinter Review
This patch addresses the feedback from Gijs and jaws, and also makes it so that it takes a "--mode" argument.

When --mode=clone, we clone the default repository. When --mode=new (default), we create a new blank profile. In either case, the profile is destroyed once Firefox shuts down.

I've also created two batch files that execute the py2exe'd makeProfile.exe with the right arguments, for even easier interaction.

Tested on my XP VM, and seemed to be OK. I think I'd like a few other people to test the package before moving forward on it though. Maybe a review would be good too.

Is there anybody here comfortable giving this a sanity review? It'll never land anywhere, but I just want to make sure there aren't any cases I'm missing.
Attachment #760004 - Attachment is obsolete: true
Just tested this on Windows XP and Windows 8 in my VMs. Seemed to run smoothly.

Any takers to give my .py a final once-over?
Flags: needinfo?
Reporter

Comment 11

6 years ago
Comment on attachment 760452 [details] [diff] [review]
makeProfile.py script

<snip>
>kExePath = os.path.join(os.path.dirname(os.path.realpath(sys.argv[0])), "firefox", "firefox.exe")

If you think you or anyone will ever use this again, stick a comment above this explaining that you're looking for a sibling directory to this file, called 'firefox', which has a 'firefox.exe' in it.


>def isFirefoxRunning():
>  """Returns true if firefox.exe is running."""
>  pipe = Popen("wmic process get description", shell=True, stdout=PIPE).stdout
>  while True:
>    s = pipe.readline().strip()
>
>    if s == "firefox.exe":
>      pipe.close()
>      return True
>
>    if not s:
>      break
>
>  pipe.close()
>  return False

In theory, pipe read calls can probably throw? Would it make sense to use try finally here and pipe.close in finally clause? I mean, probably. But is anything going to break like this? Probably not. The bonuses of throwaway code! ;-)

>def destroyProfile(newProfileDir):
>  """Erases the cloned profile and restores the backed-up profiles.ini"""
>  # Destroy the profile dir
>  shutil.rmtree(newProfileDir)
>  # And restore the backup profiles.ini
>  os.remove(kIniPath)
>  shutil.copy(kIniBackupPath, kIniPath)
>  os.remove(kIniBackupPath)
>

OK, actually, thinking about this some more, I have a slight change of heart. What if the user installs an add-on or has to restart for some other reason? Right now we always blow away their profile as soon as the browser exits. I'm sorry I didn't think of this sooner, but would it be doable to have a separate executable to 'clean up' and not destroy the profile immediately on exit? That seems safer to me. I guess what you'd want to do is pick predictable names for the copy and the new profile, and make the script reuse the profiles if they exist, but create/copy them if they do not, and then have a separate thing (can probably just be a bash script that does: rd /s /q profiles) to clean up?
That's a good call, Gijs. Let's chuck the assumption that we can clean the profiles after Firefox closes. I'll make a separate batch file to do that when the user finishes the tests.
Flags: needinfo?
Posted file Final script (obsolete) —
Ok, here's the final makeProfile.py script. I've added a "clean" mode that restores the profiles.ini and destroys any test profiles.

I've also made it so that repeated calls with --mode=clone or --mode=new will run the *same* profiles, so if the user needs to restart for some reason, they get persistence. Then --mode=clean blows those new profiles away.
Attachment #760452 - Attachment is obsolete: true
Should have removed the erroneous documentation at the top.

Anyhow, we're ready to roll here once we've got a build we're happy with.
Attachment #761128 - Attachment is obsolete: true

Comment 15

6 years ago
Any updates on this?
(In reply to Mary Trombley from comment #15)
> Any updates on this?

Great timing! Check your inbox. :)
Added the screenshots of current and Australis Firefox with 4 add-ons installed. (Adblock plus, Pocket, Firebug, Print edit)
Those screenshots were added to the ZIP. The testing build is done, and includes the following patches that have not yet landed on UX:

Bug 875488 - New toolbar / menu panel icons
Bug 877748 - Make it possible open the library with a given hierarchy
Bug 855805 - Create the bookmarks widget with subview
Bug 883268 - Disable sync for user research build

Here's the try build that I used:

https://tbpl.mozilla.org/?tree=Try&rev=4b4e5278be58

The link to the UR Build (the one that the researchers should try, and participants should download) is here:

http://people.mozilla.com/~mconley2/URBuild/www/
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.