Closed Bug 1192309 Opened 4 years ago Closed 4 years ago

Allow Firefox UI tests to run on try on testers

Categories

(Release Engineering :: General, defect)

defect
Not set

Tracking

(firefox43 fixed)

RESOLVED FIXED
Tracking Status
firefox43 --- fixed

People

(Reporter: armenzg, Assigned: armenzg)

References

Details

(Whiteboard: [leave-open])

Attachments

(4 files, 2 obsolete files)

My latest pushes:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3014b30f54ad 64-bit platforms
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c8f99952c3d 32-bit platforms

All jobs have not completed, however, we can see that the Linux 64 _test_ machines (You can ignore the syntax error) are capable of running the Firefox UI tests unlike the Linux 64 _build_ machines.
Working:
* 64-bit platforms: https://treeherder.mozilla.org/#/jobs?repo=try&revision=03abd4caa54e
* 32-bit platforms: https://treeherder.mozilla.org/#/jobs?repo=try&revision=20bbdfa06139

No crashes on L64. No missing libraries on L32.

I will have to find git.exe on the Windows testers.
Depends on: 1192525
Depends on: 1193968
The possibility to limit locales was simply to prevent running out of time on the hijacked desktop jobs.
Attachment #8647167 - Flags: review?(bhearsum)
This patch is to simply track what I did to changed the options which buildbot would call a script with.
Attachment #8647167 - Flags: review?(bhearsum) → review+
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [leave-open]
Comment on attachment 8647518 [details] [diff] [review]
[mozharness] generic testers config + move builders config

Review of attachment 8647518 [details] [diff] [review]:
-----------------------------------------------------------------

I really, really don't like "configs" with so much interpretation going on, but I'm not going to r- on that since I'm not the owner of these.

::: testing/mozharness/configs/releng_infra_configs/testers.py
@@ +8,5 @@
> +
> +external_tools_path = os.path.join(
> +    os.path.abspath(os.path.dirname(os.path.dirname(mozharness.__file__))),
> +    'external_tools',
> +)

TIL about .__file__ on modules...neat!

@@ +67,5 @@
> +        "http://pypi.pub.build.mozilla.org/pub",
> +    ],
> +    'pip_index': False,
> +    'virtualenv_path': 'venv',
> +})

Why is this done here, rather than above? What amkes find_links, pip_index, and virtualenv_path special such that they only need to be added to the running platform?
(In reply to Ben Hearsum (:bhearsum) from comment #8)
> Comment on attachment 8647518 [details] [diff] [review]
> [mozharness] generic testers config + move builders config
> 
> Review of attachment 8647518 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I really, really don't like "configs" with so much interpretation going on,
> but I'm not going to r- on that since I'm not the owner of these.
> 
Thank you.
It is mainly to reduce the complexity when calling these scripts.

> ::: testing/mozharness/configs/releng_infra_configs/testers.py
> @@ +8,5 @@
> > +
> > +external_tools_path = os.path.join(
> > +    os.path.abspath(os.path.dirname(os.path.dirname(mozharness.__file__))),
> > +    'external_tools',
> > +)
> 
> TIL about .__file__ on modules...neat!
> 
> @@ +67,5 @@
> > +        "http://pypi.pub.build.mozilla.org/pub",
> > +    ],
> > +    'pip_index': False,
> > +    'virtualenv_path': 'venv',
> > +})
> 
> Why is this done here, rather than above? What amkes find_links, pip_index,
> and virtualenv_path special such that they only need to be added to the
> running platform?

These values apply to any of the platforms.
I could have put them in a new config called generic_values.py and do something like this:
--cfg generic_values.py ---cfg releng_infra_configs/testers.py --cfg releng_infra_configs/linux64.py

I could have also added that block for each platform, however, this was less lines of code :)
Fix bitrot on hijacking patch.
Attachment #8647170 - Attachment is obsolete: true
Comment on attachment 8647518 [details] [diff] [review]
[mozharness] generic testers config + move builders config

Review of attachment 8647518 [details] [diff] [review]:
-----------------------------------------------------------------

r+, but you own this file, not me :)
Attachment #8647518 - Flags: review?(bhearsum) → review+
Depends on: 1196808
After the Git deployment, I can now see that hgtool is not working well on Windows.
It could be that the HOME env variable is None instead of a string.
I added the env and have pushed it.
HOME is needed because of hg help share [2]



[1]
https://treeherder.mozilla.org/#/jobs?repo=try&revision=43e0c395c542
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/armenzg@mozilla.com-43e0c395c542/try-win64/try_win8_64_test-mochitest-1-bm119-tests1-windows-build1323.txt.gz
08:37:19     INFO - retry: Calling _get_revision with args: (<mozharness.base.vcs.hgtool.HgtoolVCS object at 0x01D6B230>, 'C:\\slave\\test\\build\\tools'), kwargs: {}, attempt #1
08:37:19     INFO - Running command: ['c:/mozilla-build/python27/python.exe', 'C:\\slave\\test\\scripts\\external_tools\\hgtool.py', '-r', 'FIREFOX_40_0b6_RELEASE_RUNTIME', 'http://hg.mozilla.org/build/tools', 'C:\\slave\\test\\build\\tools']
08:37:19     INFO - Copy/paste: c:/mozilla-build/python27/python.exe C:\slave\test\scripts\external_tools\hgtool.py -r FIREFOX_40_0b6_RELEASE_RUNTIME http://hg.mozilla.org/build/tools C:\slave\test\build\tools
08:37:19     INFO - Using env: {'HOME': None,
08:37:19     INFO -  'PATH': 'C:\\Program Files (x86)\\NVIDIA Corporation\\PhysX\\Common;C:\\windows\\system32;C:\\windows;C:\\windows\\System32\\Wbem;C:\\windows\\System32\\WindowsPowerShell\\v1.0\\;C:\\mozilla-build\\python27;C:\\mozilla-build\\python27\\Scripts;C:\\mozilla-build\\msys\\bin;C:\\mozilla-build\\vim\\vim72;C:\\mozilla-build\\wget;C:\\mozilla-build\\info-zip;C:\\CoreUtils\\bin;C:\\mozilla-build\\buildbotve\\scripts;C:\\mozilla-build\\hg',
08:37:19     INFO -  'SYSTEMROOT': 'C:\\windows'}
08:37:19     INFO - retry: attempt #1 caught exception: environment can only contain strings
08:37:19     INFO - retry: Failed, sleeping 60 seconds before retrying

[2] http://hg.mozilla.org/mozilla-central/file/29b2df16e961/testing/mozharness/mozharness/base/vcs/hgtool.py#l81
Bug 1192309 - Add HOME env for Windows. r=bhearsum
Attachment #8651826 - Flags: review?(bhearsum)
Comment on attachment 8651826 [details]
MozReview Request: Bug 1192309 - hgtool.py should set empty strings for Windows instead of None. r=jlund

I'm not sure I'm the right person to review this, I don't understand the implications of changing this file, or of setting HOME on Windows...can you try to find someone else?
Attachment #8651826 - Flags: review?(bhearsum)
That's OK Ben.

We're now a bit closer. We're past the hgtool issue and we now encounter a new one:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/armenzg@mozilla.com-cb88f1c4d3b6/try-win64/try_win8_64_test-mochitest-1-bm109-tests1-windows-build1506.txt.gz

I will try on the loaner and see what I discover.

22:05:36     INFO - Calling ['C:\\slave\\test\\build\\venv\\Scripts\\firefox-ui-update', '--installer', 'C:\\slave\\test\\build\\Firefox%20Setup%2040.0b1.exe', '--gecko-log=-', '--address=localhost:2828', '--update-channel', 'beta-localtest'] with output_timeout 300
ProcessManager UNABLE to use job objects to manage child processes
['C:\\slave\\test\\build\\venv\\Scripts\\firefox-ui-update', '--installer', 'C:\\slave\\test\\build\\Firefox%20Setup%2040.0b1.exe', '--gecko-log=-', '--address=localhost:2828', '--update-channel', 'beta-localtest']
22:05:36    ERROR - caught OS error 22: The requested operation requires elevation. while running ['C:\\slave\\test\\build\\venv\\Scripts\\firefox-ui-update', '--installer', 'C:\\slave\\test\\build\\Firefox%20Setup%2040.0b1.exe', '--gecko-log=-', '--address=localhost:2828', '--update-channel', 'beta-localtest']
Exception AttributeError: "'Process' object has no attribute '_handle'" in <bound method Process.__del__ of <mozprocess.processhandler.Process object at 0x02A30F10>> ignored
22:05:36     INFO - Internally this is the command fx-ui-updates executed
22:05:36     INFO - C:\slave\test\build\venv\Scripts\firefox-ui-update --installer C:\slave\test\build\Firefox%20Setup%2040.0b1.exe --gecko-log=- --address=localhost:2828 --update-channel beta-localtest
22:05:36  WARNING - FAIL: firefox-ui-update has failed.
Comment on attachment 8651826 [details]
MozReview Request: Bug 1192309 - hgtool.py should set empty strings for Windows instead of None. r=jlund

Bug 1192309 - hgtool.py should set empty strings for Windows instead of None. r=jlund
Attachment #8651826 - Attachment description: MozReview Request: Bug 1192309 - Add HOME env for Windows. r=bhearsum → MozReview Request: Bug 1192309 - hgtool.py should set empty strings for Windows instead of None. r=jlund
Attachment #8651826 - Flags: review?(jlund)
Depends on: 1198448
Comment on attachment 8651826 [details]
MozReview Request: Bug 1192309 - hgtool.py should set empty strings for Windows instead of None. r=jlund

https://reviewboard.mozilla.org/r/16995/#review15405

Ship It!

::: testing/mozharness/mozharness/base/vcs/hgtool.py:79
(Diff revision 2)
>              if 'SYSTEMROOT' not in env:

I guess you could have

```python
config {
    'env': {'SYSTEMROOT': None}   
}
```

in your self.config which would sneak through this condition, but I suppose if you are that explicit in your config, you will actually want it to be None and not ''
Attachment #8651826 - Flags: review?(jlund) → review+
Comment on attachment 8651826 [details]
MozReview Request: Bug 1192309 - hgtool.py should set empty strings for Windows instead of None. r=jlund

Bug 1192309 - hgtool.py should set empty strings for Windows instead of None. r=jlund
Bug 1198448 - The Firefox UI update tests should call a different entry point. r=chmanchester

When setting up the Firefox UI tests repository, it generates two binaries, one of them
called firefox-ui-update. Calling this binary on Windows triggers the UAC prompt for the
Release Engineering Windows test machines.

If instead we use Python to call a script that calls the same functionality we don't
get this UAC prompt.

We also move around some pre action checks to the right place. This allows us to not need to
specify --firefox-ui-branch when we're not running the checkout action.
Attachment #8656711 - Flags: review?(cmanchester)
chmanchester: this shows the code working:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e08a06ee11be
Comment on attachment 8656711 [details]
MozReview Request: Bug 1198448 - The Firefox UI update tests should call a different entry point. r=chmanchester

https://reviewboard.mozilla.org/r/18235/#review16339

This looks good (although I don't think update_script.py has landed yet, so may depend on that).

::: testing/mozharness/scripts/firefox_ui_updates.py:151
(Diff revision 1)
> -        assert ('update_verify_config' in self.config or
> -                self.installer_url or self.installer_path,
> -                'Either specify --update-verify-config, --installer-url or --installer-path.')
> +        assert 'update_verify_config' in self.config or \
> +                self.installer_url or self.installer_path, \
> +                'Either specify --update-verify-config, --installer-url or --installer-path.'

The over way to solve this would be another set of parens around the first two lines.
Attachment #8656711 - Flags: review?(cmanchester) → review+
Comment on attachment 8656711 [details]
MozReview Request: Bug 1198448 - The Firefox UI update tests should call a different entry point. r=chmanchester

I will rebase this patch on top of the recent changes by whimboo.
On Tuesday I will re-test it.
I will ask whimboo to review it since he's touching this area of code.
Attachment #8656711 - Attachment is obsolete: true
This bug will resolve once this PR lands:
https://github.com/mozilla/firefox-ui-tests/pull/233
Merged:
https://github.com/mozilla/firefox-ui-tests/commit/310ff2d40229aac20a346a8597dda42dd7f225be
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.