1337827 - [one-click loaner] Setup mach binary in one-click loaner even when choosing option 1 in the wizard

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Andrew Halberstadt [:ahal]]

Currently the 'run-wizard' script will only set up mach if you choose option 2. This means if you choose option 1, then Ctrl-C and want to start over, you won't be able to without re-running the wizard.

We should catch Ctrl-C in the run-wizard script, check to see if 'mach' exists, and link it if it does.

Comment 1

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1337827 - [one-click-loaner] Print error message if mach binary wasn't found

Review commit: https://reviewboard.mozilla.org/r/111226/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/111226/

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8835493 [details]
Bug 1337827 - [one-click-loaner] Print error message if mach binary wasn't found

I am not familiar with this code, nor do I really follow the changes- I would prefer if someone else could review this as a rubber stamp is all I can give (and would be happy to)

Comment 3

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1337827 - [one-click-loaner] Refactor mach environment setup into a standalone function

Review commit: https://reviewboard.mozilla.org/r/111572/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/111572/

Comment 4

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1337827 - [one-click-loaner] Always set up mach environment when calling 'run-mozharness' (if possible)

This is a minor refactor that aims to always attempt to set up 'mach' after running the 'run-mozharness' script.
We put it in a 'finally' block so we'll even do this if the user presses Ctrl-C or there was an exception
in the test harness. Importantly, this will be set up regardless of whether the user chooses "Option 1" or
"Option 2" at the wizard prompt.

The reason for this change is mostly 'might as well'. If it can save some users confusion, then it is worthwhile.

Review commit: https://reviewboard.mozilla.org/r/111574/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/111574/

Comment 5

[Andrew Halberstadt [:ahal]]

Comment on attachment 8835493 [details]
Bug 1337827 - [one-click-loaner] Print error message if mach binary wasn't found

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/111226/diff/1-2/

Comment 6

[Andrew Halberstadt [:ahal]]

No worries, I got Maja to volunteer. And you're right it was kind of tricky to see what the change was about.

Maja: I split it up and added a more detailed commit message so it's hopefully easier to follow now. I'm also happy to explain things further.. just ping me on irc if you want more clarification.

Comment 7

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Comment on attachment 8835493 [details]
Bug 1337827 - [one-click-loaner] Print error message if mach binary wasn't found

https://reviewboard.mozilla.org/r/111226/#review113678

Comment 8

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Comment on attachment 8836126 [details]
Bug 1337827 - [one-click-loaner] Refactor mach environment setup into a standalone function

https://reviewboard.mozilla.org/r/111572/#review113680

Comment 9

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Comment on attachment 8836127 [details]
Bug 1337827 - [one-click-loaner] Always set up mach environment when calling 'run-mozharness' (if possible)

https://reviewboard.mozilla.org/r/111574/#review113682

Comment 10

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

(In reply to Andrew Halberstadt [:ahal] from comment #6)
> No worries, I got Maja to volunteer. And you're right it was kind of tricky
> to see what the change was about.
> 
> Maja: I split it up and added a more detailed commit message so it's
> hopefully easier to follow now. I'm also happy to explain things further..
> just ping me on irc if you want more clarification.

It all made sense, thanks. Sorry for the delayed reply.

Comment 11

[Andrew Halberstadt [:ahal]]

Thanks, no problem! A turn around time of ~1 workday is hardly a delay

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9ba825a65a34
[one-click-loaner] Refactor mach environment setup into a standalone function r=maja_zf
https://hg.mozilla.org/integration/autoland/rev/b3c43c86a86b
[one-click-loaner] Print error message if mach binary wasn't found r=maja_zf
https://hg.mozilla.org/integration/autoland/rev/8c97b432e7c5
[one-click-loaner] Always set up mach environment when calling 'run-mozharness' (if possible) r=maja_zf

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9ba825a65a34
https://hg.mozilla.org/mozilla-central/rev/b3c43c86a86b
https://hg.mozilla.org/mozilla-central/rev/8c97b432e7c5