Skip to content

Commit

Permalink
Setup logging earlier in the wpt run startup.
Browse files Browse the repository at this point in the history
Curerntly we wait until we're setting up wptrunner kwargs to create the logger.
But that's pretty silly; we should set up the logger first thing and then use it
consistently to log during the setup phase.
  • Loading branch information
jgraham committed Nov 9, 2018
1 parent 80d76f8 commit 47f0061
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 25 deletions.
4 changes: 2 additions & 2 deletions tools/wpt/browser.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ def install_prefs(self, binary, dest=None, channel=None):

url = self.get_profile_bundle_url(version, channel)

print("Installing test prefs from %s" % url)
logger.info("Installing test prefs from %s" % url)
try:
extract_dir = tempfile.mkdtemp()
unzip(get(url).raw, dest=extract_dir)
Expand All @@ -294,7 +294,7 @@ def install_prefs(self, binary, dest=None, channel=None):
finally:
shutil.rmtree(extract_dir)
else:
print("Using cached test prefs from %s" % dest)
logger.info("Using cached test prefs from %s" % dest)

return dest

Expand Down
42 changes: 24 additions & 18 deletions tools/wpt/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ def setup_kwargs(self, kwargs):
logger.info("""Can't find certutil, certificates will not be checked.
Consider installing certutil via your OS package manager or directly.""")
else:
print("Using certutil %s" % certutil)
logger.info("Using certutil %s" % certutil)

kwargs["certutil_binary"] = certutil

Expand All @@ -208,15 +208,15 @@ def setup_kwargs(self, kwargs):
install = self.prompt_install("geckodriver")

if install:
print("Downloading geckodriver")
logger.info("Downloading geckodriver")
webdriver_binary = self.browser.install_webdriver(dest=self.venv.bin_path)
else:
print("Using webdriver binary %s" % webdriver_binary)
logger.info("Using webdriver binary %s" % webdriver_binary)

if webdriver_binary:
kwargs["webdriver_binary"] = webdriver_binary
else:
print("Unable to find or install geckodriver, skipping wdspec tests")
logger.info("Unable to find or install geckodriver, skipping wdspec tests")
kwargs["test_types"].remove("wdspec")

if kwargs["prefs_root"] is None:
Expand Down Expand Up @@ -253,10 +253,10 @@ def setup_kwargs(self, kwargs):
install = self.prompt_install("chromedriver")

if install:
print("Downloading chromedriver")
logger.info("Downloading chromedriver")
webdriver_binary = self.browser.install_webdriver(dest=self.venv.bin_path)
else:
print("Using webdriver binary %s" % webdriver_binary)
logger.info("Using webdriver binary %s" % webdriver_binary)

if webdriver_binary:
kwargs["webdriver_binary"] = webdriver_binary
Expand Down Expand Up @@ -287,10 +287,10 @@ def setup_kwargs(self, kwargs):
install = self.prompt_install("chromedriver")

if install:
print("Downloading chromedriver")
logger.info("Downloading chromedriver")
webdriver_binary = self.browser.install_webdriver(dest=self.venv.bin_path)
else:
print("Using webdriver binary %s" % webdriver_binary)
logger.info("Using webdriver binary %s" % webdriver_binary)

if webdriver_binary:
kwargs["webdriver_binary"] = webdriver_binary
Expand All @@ -310,10 +310,10 @@ def setup_kwargs(self, kwargs):
install = self.prompt_install("operadriver")

if install:
print("Downloading operadriver")
logger.info("Downloading operadriver")
webdriver_binary = self.browser.install_webdriver(dest=self.venv.bin_path)
else:
print("Using webdriver binary %s" % webdriver_binary)
logger.info("Using webdriver binary %s" % webdriver_binary)

if webdriver_binary:
kwargs["webdriver_binary"] = webdriver_binary
Expand Down Expand Up @@ -454,18 +454,12 @@ def setup_kwargs(self, kwargs):
}


def setup_wptrunner(venv, prompt=True, install_browser=False, **kwargs):
from wptrunner import wptrunner, wptcommandline
def setup_logging(kwargs):
import mozlog
from wptrunner import wptrunner

global logger

kwargs = utils.Kwargs(kwargs.iteritems())

product_parts = kwargs["product"].split(":")
kwargs["product"] = product_parts[0]
sub_product = product_parts[1:]

# Use the grouped formatter by default where mozlog 3.9+ is installed
if hasattr(mozlog.formatters, "GroupingFormatter"):
default_formatter = "grouped"
Expand All @@ -474,6 +468,16 @@ def setup_wptrunner(venv, prompt=True, install_browser=False, **kwargs):
wptrunner.setup_logging(kwargs, {default_formatter: sys.stdout})
logger = wptrunner.logger


def setup_wptrunner(venv, prompt=True, install_browser=False, **kwargs):
from wptrunner import wptcommandline

kwargs = utils.Kwargs(kwargs.iteritems())

product_parts = kwargs["product"].split(":")
kwargs["product"] = product_parts[0]
sub_product = product_parts[1:]

check_environ(kwargs["product"])
args_general(kwargs)

Expand Down Expand Up @@ -516,6 +520,8 @@ def setup_wptrunner(venv, prompt=True, install_browser=False, **kwargs):


def run(venv, **kwargs):
setup_logging(kwargs)

# Remove arguments that aren't passed to wptrunner
prompt = kwargs.pop("prompt", True)
install_browser = kwargs.pop("install_browser", False)
Expand Down
7 changes: 6 additions & 1 deletion tools/wpt/tests/test_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ def install_requirements(self, requirements_path):
shutil.rmtree(venv.path)


@pytest.fixture(scope="module")
def logger():
run.setup_logging({})


@pytest.mark.parametrize("platform", ["Windows", "Linux", "Darwin"])
def test_check_environ_fail(platform):
m_open = mock.mock_open(read_data=b"")
Expand All @@ -53,7 +58,7 @@ def test_check_environ_fail(platform):


@pytest.mark.parametrize("product", product_list)
def test_setup_wptrunner(venv, product):
def test_setup_wptrunner(venv, logger, product):
parser = run.create_parser()
kwargs = vars(parser.parse_args(["--channel=nightly", product]))
kwargs["prompt"] = False
Expand Down
5 changes: 1 addition & 4 deletions tools/wptrunner/wptrunner/testloader.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,13 @@
manifest = None
manifest_update = None
download_from_github = None
manifest_log = None

def do_delayed_imports():
# This relies on an already loaded module having set the sys.path correctly :(
global manifest, manifest_update, download_from_github, manifest_log
global manifest, manifest_update, download_from_github
from manifest import manifest
from manifest import update as manifest_update
from manifest.download import download_from_github
from manifest import log as manifest_log


class TestChunker(object):
Expand Down Expand Up @@ -406,7 +404,6 @@ def create_manifest(self, manifest_path, tests_path, url_base="/"):
def update_manifest(self, manifest_path, tests_path, url_base="/",
recreate=False, download=False):
self.logger.info("Updating test manifest %s" % manifest_path)
manifest_log.setup()

json_data = None
if download:
Expand Down

0 comments on commit 47f0061

Please sign in to comment.