Skip to content

Commit

Permalink
Bug 1632429: Migrate |./mach try| to python 3; r=rstewart,ahal
Browse files Browse the repository at this point in the history
|./mach try| subcommands are now compatible with both python 2 and 3.

Hand-tested with many combinations of subcommand and subcommand flags.

Updates tryselect unit tests to use Python 3.

Differential Revision: https://phabricator.services.mozilla.com/D73398
  • Loading branch information
Mitchell Hentges committed May 6, 2020
1 parent 7c7f873 commit 1bfa1d4
Show file tree
Hide file tree
Showing 19 changed files with 50 additions and 35 deletions.
1 change: 0 additions & 1 deletion mach
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ py2commands="
test
test-info
tps-build
try
valgrind-test
visualmetrics
web-platform-tests
Expand Down
4 changes: 3 additions & 1 deletion python/mozboot/mozboot/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import hashlib
import os

import six

here = os.path.join(os.path.dirname(__file__))


Expand All @@ -32,7 +34,7 @@ def get_state_dir(srcdir=False):
srcdir = os.path.abspath(MozbuildObject.from_environment(cwd=here).topsrcdir)
# Shortening to 12 characters makes these directories a bit more manageable
# in a terminal and is more than good enough for this purpose.
srcdir_hash = hashlib.sha256(srcdir).hexdigest()[:12]
srcdir_hash = hashlib.sha256(six.ensure_binary(srcdir)).hexdigest()[:12]

state_dir = os.path.join(state_dir, 'srcdirs', '{}-{}'.format(
os.path.basename(srcdir), srcdir_hash))
Expand Down
4 changes: 2 additions & 2 deletions python/mozbuild/mozbuild/chunkify.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ def split_evenly(n, chunks):
raise ChunkingError("Number of chunks is greater than number")
if n % chunks == 0:
# Either we can evenly split or only 1 chunk left
return [n / chunks] * chunks
return [n // chunks] * chunks
# otherwise the current chunk should be a bit larger
max_size = n / chunks + 1
max_size = n // chunks + 1
return [max_size] + split_evenly(n - max_size, chunks - 1)


Expand Down
2 changes: 1 addition & 1 deletion taskcluster/ci/source-test/python.yml
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ tryselect:
platform:
- linux1804-64/opt
- windows10-64/opt
python-version: [2]
python-version: [3]
treeherder:
symbol: try
run:
Expand Down
2 changes: 1 addition & 1 deletion testing/mozharness/configs/unittests/win_unittest.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@
'name': 'disable windows security and maintenance notifications',
'cmd': [
'powershell', '-command',
'"&{$p=\'HKCU:SOFTWARE\Microsoft\Windows\CurrentVersion\Notifications\Settings\Windows.SystemToast.SecurityAndMaintenance\';if(!(Test-Path -Path $p)){&New-Item -Path $p -Force}&Set-ItemProperty -Path $p -Name Enabled -Value 0}"' # noqa
'"&{$p=\'HKCU:SOFTWARE\Microsoft\Windows\CurrentVersion\\Notifications\Settings\Windows.SystemToast.SecurityAndMaintenance\';if(!(Test-Path -Path $p)){&New-Item -Path $p -Force}&Set-ItemProperty -Path $p -Name Enabled -Value 0}"' # noqa
],
'architectures': ['32bit', '64bit'],
'halt_on_failure': True,
Expand Down
3 changes: 2 additions & 1 deletion testing/mozharness/scripts/android_emulator_unittest.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
import subprocess

# load modules from parent dir
sys.path.insert(1, os.path.dirname(sys.path[0]))
here = os.path.abspath(os.path.dirname(__file__))
sys.path.insert(1, os.path.dirname(here))

from mozharness.base.log import WARNING
from mozharness.base.script import BaseScript, PreScriptAction
Expand Down
1 change: 1 addition & 0 deletions tools/lint/py2.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ py2:
- tools/crashreporter/system-symbols/win/symsrv-fetch.py
- tools/github-sync
- tools/lint
- tools/tryselect
- testing/performance
extensions: ['py']
support-files:
Expand Down
3 changes: 2 additions & 1 deletion tools/tryselect/mach_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import os
import sys

import six
from mach.decorators import (
CommandProvider,
Command,
Expand Down Expand Up @@ -156,7 +157,7 @@ def handle_presets(self, preset_action, save, preset, **kwargs):
def handle_try_config(self, **kwargs):
from tryselect.util.dicttools import merge
kwargs.setdefault('try_config', {})
for cls in self.parser.task_configs.itervalues():
for cls in six.itervalues(self.parser.task_configs):
try_config = cls.try_config(**kwargs)
if try_config is not None:
kwargs['try_config'] = merge(kwargs['try_config'], try_config)
Expand Down
5 changes: 3 additions & 2 deletions tools/tryselect/push.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import os
import sys

import six
from mozboot.util import get_state_dir
from mozbuild.base import MozbuildObject
from mozversioncontrol import get_repository_object, MissingVCSExtension
Expand Down Expand Up @@ -161,8 +162,8 @@ def push_to_try(method, msg, try_task_config=None,
if files_to_change:
for path, content in files_to_change.items():
path = os.path.join(vcs.path, path)
with open(path, 'w') as fh:
fh.write(content)
with open(path, 'wb') as fh:
fh.write(six.ensure_binary(content))
changed_files.append(path)

try:
Expand Down
2 changes: 1 addition & 1 deletion tools/tryselect/selectors/coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
build = MozbuildObject.from_environment(cwd=here)
vcs = get_repository_object(build.topsrcdir)

root_hash = hashlib.sha256(os.path.abspath(build.topsrcdir)).hexdigest()
root_hash = hashlib.sha256(six.ensure_binary(os.path.abspath(build.topsrcdir))).hexdigest()
cache_dir = os.path.join(get_state_dir(), 'cache', root_hash, 'chunk_mapping')
if not os.path.isdir(cache_dir):
os.makedirs(cache_dir)
Expand Down
15 changes: 10 additions & 5 deletions tools/tryselect/selectors/fuzzy.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@
import os
import platform
import subprocess
import six
import sys
from distutils.spawn import find_executable
from distutils.version import StrictVersion
from six.moves import input

from mozbuild.base import MozbuildObject
from mozbuild.util import ensure_subprocess_env
Expand Down Expand Up @@ -187,7 +189,7 @@ def should_force_fzf_update(fzf_bin):
sys.exit(1)

# Some fzf versions have extra, e.g 0.18.0 (ff95134)
fzf_version = fzf_version.split()[0]
fzf_version = six.ensure_text(fzf_version.split()[0])

# 0.20.0 introduced passing selections through a temporary file,
# which is good for large ctrl-a actions.
Expand Down Expand Up @@ -238,7 +240,7 @@ def get_fzf():

return fzf_bin

install = raw_input("Could not detect fzf, install it now? [y/n]: ")
install = input("Could not detect fzf, install it now? [y/n]: ")
if install.lower() != 'y':
return

Expand Down Expand Up @@ -270,7 +272,8 @@ def run_fzf(cmd, tasks):
env = dict(os.environ)
env.update({'PYTHONPATH': os.pathsep.join([p for p in sys.path if 'requests' in p])})
proc = subprocess.Popen(
cmd, stdout=subprocess.PIPE, stdin=subprocess.PIPE, env=ensure_subprocess_env(env)
cmd, stdout=subprocess.PIPE, stdin=subprocess.PIPE,
env=ensure_subprocess_env(env), universal_newlines=True
)
out = proc.communicate('\n'.join(tasks))[0].splitlines()

Expand Down Expand Up @@ -312,14 +315,16 @@ def run(update=False, query=None, intersect_query=None, try_config=None, full=Fa
make_trimmed_taskgraph_cache(graph_cache, dep_cache, target_file=target_set)

if not full and not disable_target_task_filter:
all_tasks = filter(filter_tasks_by_blacklist, all_tasks)
# Put all_tasks into a list because it's used multiple times, and "filter()"
# returns a consumable iterator.
all_tasks = list(filter(filter_tasks_by_blacklist, all_tasks))

if test_paths:
all_tasks = filter_tasks_by_paths(all_tasks, test_paths)
if not all_tasks:
return 1

key_shortcuts = [k + ':' + v for k, v in fzf_shortcuts.iteritems()]
key_shortcuts = [k + ':' + v for k, v in six.iteritems(fzf_shortcuts)]
base_cmd = [
fzf, '-m',
'--bind', ','.join(key_shortcuts),
Expand Down
4 changes: 3 additions & 1 deletion tools/tryselect/selectors/release.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ def run(
'{}.py'.format(migration.replace('-', '_')),
)
migration_config = {}
execfile(migration_path, migration_config, migration_config)
with open(migration_path) as f:
code = compile(f.read(), migration_path, "exec")
exec(code, migration_config, migration_config)
for (path, from_, to) in migration_config['config']['replacements']:
if path in files_to_change:
contents = files_to_change[path]
Expand Down
4 changes: 2 additions & 2 deletions tools/tryselect/selectors/scriptworker.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,15 @@ class ScriptworkerParser(BaseTryParser):
[
["task_type"],
{
"choices": ["list"] + TASK_TYPES.keys(),
"choices": ["list"] + list(TASK_TYPES.keys()),
"metavar": "TASK-TYPE",
"help": "Scriptworker task types to run. (Use `list` to show possibilities)",
},
],
[
["--release-type"],
{
"choices": ["nightly"] + RELEASE_TO_BRANCH.keys(),
"choices": ["nightly"] + list(RELEASE_TO_BRANCH.keys()),
"default": "beta",
"help": "Release type to run",
},
Expand Down
13 changes: 7 additions & 6 deletions tools/tryselect/selectors/syntax.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from collections import defaultdict

import mozpack.path as mozpath
import six
from moztest.resolve import TestResolver

from ..cli import BaseTryParser
Expand Down Expand Up @@ -207,7 +208,7 @@ def parse(self, tokens):

def consume(self):
try:
self.token = self.tokens.next()
self.token = next(self.tokens)
except StopIteration:
self.token = (self.EOF, None)

Expand Down Expand Up @@ -390,7 +391,7 @@ def calc_try_syntax(self, platforms, tests, talos, jobs, builds, paths_by_flavor

suites = tests if not intersection else {}
paths = set()
for flavor, flavor_tests in paths_by_flavor.iteritems():
for flavor, flavor_tests in six.iteritems(paths_by_flavor):
suite = self.flavor_suites[flavor]
if suite not in suites and (not intersection or suite in tests):
for job_name in self.flavor_jobs[flavor]:
Expand Down Expand Up @@ -450,7 +451,7 @@ def calc_try_syntax(self, platforms, tests, talos, jobs, builds, paths_by_flavor
parts.append("--try-test-paths %s" % " ".join(sorted(paths)))

args_by_dest = {v['dest']: k for k, v in SyntaxParser.pass_through_arguments.items()}
for dest, value in extras.iteritems():
for dest, value in six.iteritems(extras):
assert dest in args_by_dest
arg = args_by_dest[dest]
action = SyntaxParser.pass_through_arguments[arg]['action']
Expand All @@ -470,11 +471,11 @@ def normalise_list(self, items, allow_subitems=False):
rv = defaultdict(list)
for item in items:
parsed = parse_arg(item)
for key, values in parsed.iteritems():
for key, values in six.iteritems(parsed):
rv[key].extend(values)

if not allow_subitems:
if not all(item == [] for item in rv.itervalues()):
if not all(item == [] for item in six.itervalues(rv)):
raise ValueError("Unexpected subitems in argument")
return rv.keys()
else:
Expand Down Expand Up @@ -598,7 +599,7 @@ def run(self, **kwargs):

if kwargs["verbose"] and paths_by_flavor:
print('The following tests will be selected: ')
for flavor, paths in paths_by_flavor.iteritems():
for flavor, paths in six.iteritems(paths_by_flavor):
print("%s: %s" % (flavor, ",".join(paths)))

if kwargs["verbose"]:
Expand Down
1 change: 0 additions & 1 deletion tools/tryselect/test/python.ini
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
[DEFAULT]
subsuite=try
skip-if = python == 3

[test_again.py]
[test_chooser.py]
Expand Down
2 changes: 2 additions & 0 deletions tools/tryselect/test/test_again.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import mozunit
import pytest

from six.moves import reload_module as reload

from tryselect import push
from tryselect.selectors import again

Expand Down
12 changes: 6 additions & 6 deletions tools/tryselect/test/test_chooser.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,30 +47,30 @@ def test_try_chooser(app):
assert response.status_code == 200

expected_output = [
"""<title>Try Chooser Enhanced</title>""",
"""<input class="filter" type="checkbox" id=windows name="build" value='{"build_platform": ["windows"]}' onchange="console.log('checkbox onchange triggered');apply();">""", # noqa
"""<input class="filter" type="checkbox" id=mochitest-browser-chrome name="test" value='{"unittest_suite": ["mochitest-browser-chrome"]}' onchange="console.log('checkbox onchange triggered');apply();">""", # noqa
b"""<title>Try Chooser Enhanced</title>""",
b"""<input class="filter" type="checkbox" id=windows name="build" value='{"build_platform": ["windows"]}' onchange="console.log('checkbox onchange triggered');apply();">""", # noqa
b"""<input class="filter" type="checkbox" id=mochitest-browser-chrome name="test" value='{"unittest_suite": ["mochitest-browser-chrome"]}' onchange="console.log('checkbox onchange triggered');apply();">""", # noqa
]

for expected in expected_output:
assert expected in response.data

response = client.post('/', data={'action': 'Cancel'})
assert response.status_code == 200
assert "You may now close this page" in response.data
assert b"You may now close this page" in response.data
assert app.tasks == []

response = client.post('/', data={'action': 'Push', 'selected-tasks': ''})
assert response.status_code == 200
assert "You may now close this page" in response.data
assert b"You may now close this page" in response.data
assert app.tasks == []

response = client.post('/', data={
'action': 'Push',
'selected-tasks': 'build-windows\ntest-windows-mochitest-e10s'
})
assert response.status_code == 200
assert "You may now close this page" in response.data
assert b"You may now close this page" in response.data
assert set(app.tasks) == set(['build-windows', 'test-windows-mochitest-e10s'])


Expand Down
4 changes: 2 additions & 2 deletions tools/tryselect/test/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ def test_filter_tasks_by_paths(patch_resolver):
tasks = ['foobar/xpcshell-1', 'foobar/mochitest', 'foobar/xpcshell']

patch_resolver(['xpcshell'], {})
assert filter_tasks_by_paths(tasks, 'dummy') == []
assert list(filter_tasks_by_paths(tasks, 'dummy')) == []

patch_resolver([], [{'flavor': 'xpcshell'}])
assert filter_tasks_by_paths(tasks, 'dummy') == ['foobar/xpcshell-1', 'foobar/xpcshell']
assert list(filter_tasks_by_paths(tasks, 'dummy')) == ['foobar/xpcshell-1', 'foobar/xpcshell']


def test_resolve_tests_by_suite(patch_resolver):
Expand Down
3 changes: 2 additions & 1 deletion tools/tryselect/util/manage_estimates.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import json
from datetime import datetime, timedelta

import six

TASK_DURATION_URL = 'https://storage.googleapis.com/mozilla-mach-data/task_duration_history.json'
GRAPH_QUANTILES_URL = 'https://storage.googleapis.com/mozilla-mach-data/machtry_quantiles.csv'
Expand Down Expand Up @@ -84,7 +85,7 @@ def download_task_history_data(cache_dir):
return

with open(graph_quantile_cache, 'w') as f:
f.write(r.content)
f.write(six.ensure_text(r.content))

with open(task_duration_tag_file, 'w') as f:
json.dump({
Expand Down

0 comments on commit 1bfa1d4

Please sign in to comment.