Skip to content

Commit

Permalink
Indented code which prints warnings for unrecognized os's.
Browse files Browse the repository at this point in the history
Previous indentation level was causing the warning to print when
the OS name happened to be one of the preferred ("normalized") os
names, which is pretty much the opposite of intended behavior.

Testing Done:
Added test_osutil.py to make sure osutil.py is behaving properly.

CI went green: https://travis-ci.org/gmalmquist/pants/builds/77405988

Bugs closed: 2075

Reviewed at https://rbcommons.com/s/twitter/r/2713/
  • Loading branch information
gmalmquist committed Aug 26, 2015
1 parent 52995c5 commit 81e2034
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/python/pants/util/osutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ def normalize_os_name(os_name):
for proper_name, aliases in OS_ALIASES.items():
if os_name in aliases:
return proper_name
logger.warning('Unknown operating system name: {bad}, known names are: {known}'
.format(bad=os_name, known=', '.join(sorted(known_os_names()))))
logger.warning('Unknown operating system name: {bad}, known names are: {known}'
.format(bad=os_name, known=', '.join(sorted(known_os_names()))))
return os_name


Expand Down
9 changes: 9 additions & 0 deletions tests/python/pants_test/util/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ target(
':fileutil',
':memo',
':meta',
':osutil',
':strutil',
':xml_parser',
]
Expand Down Expand Up @@ -50,6 +51,14 @@ python_tests(
]
)

python_tests(
name = 'osutil',
sources = ['test_osutil.py'],
dependencies = [
'src/python/pants/util:osutil',
]
)

python_tests(
name = 'memo',
sources = ['test_memo.py'],
Expand Down
54 changes: 54 additions & 0 deletions tests/python/pants_test/util/test_osutil.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# coding=utf-8
# Copyright 2014 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
unicode_literals, with_statement)

import logging
import unittest
from contextlib import contextmanager

from pants.util.osutil import OS_ALIASES, known_os_names, normalize_os_name


class OsutilTest(unittest.TestCase):

class WarningRecorder(object):
"""Simple logging handler to record warnings."""

def __init__(self):
self.warning_list = []
self.level = logging.WARNING

def handle(self, record):
self.warning_list.append('{}: {}'.format(record.name, record.getMessage()))

@contextmanager
def warnings(self):
handler = self.WarningRecorder()
logging.getLogger('').addHandler(handler)
yield handler.warning_list

def test_alias_normalization(self):
for normal_os, aliases in OS_ALIASES.items():
for alias in aliases:
self.assertEqual(normal_os, normalize_os_name(alias))

def test_keys_in_aliases(self):
for key in OS_ALIASES.keys():
self.assertIn(key, known_os_names())

def test_no_warnings_on_known_names(self):
for name in known_os_names():
with self.warnings() as warning_list:
normalize_os_name(name)
self.assertEqual(0, len(warning_list),
'Recieved unexpected warnings: {}'.format(warning_list))

def test_warnings_on_unknown_names(self):
name = 'I really hope no one ever names an operating system with this string.'
with self.warnings() as warning_list:
normalize_os_name(name)
self.assertEqual(1, len(warning_list),
'Expected exactly one warning, but got: {}'.format(warning_list))

0 comments on commit 81e2034

Please sign in to comment.