launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Thanks for doing this.  Not a full review (yet), but a note about the changes in test_js.py:

171	+@nottest
172	+def get_failed_tests_message(results):
173	+    """Return a readable error message with the list of the failed tests.
174	+
175	+    Given a YUI3 results_ json object, return a readable error message.
176	+
177	+    .. _results: http://yuilibrary.com/yui/docs/test/
178	+    """
179	+    result = []
180	+    suites = [item for item in results.values() if isinstance(item, dict)]
181	+    for suite in suites:
182	+        if suite['failed'] != 0:
183	+            tests = [item for item in suite.values()
184	+                     if isinstance(item, dict)]
185	+            for test in tests:
186	+                if test['result'] != 'pass':
187	+                    result.append('\n%s.%s: %s\n' % (
188	+                        suite['name'], test['name'], test['message']))
189	+    return ''.join(result)

This is pretty hard to follow, and it indents into dizzying heights.  Filtering in steps may be clearer:


def extract_dicts(results):
    """List those items from `results` that are dicts."""
    return [
        item for item in results.values() if isintance(item, dict)]


def list_failures(suite):
    """List those tests from `suite` that did not pass."""
    has_failed = lambda test: test['result'] != 'pass'
    return list(filter(has_failed, extract_dicts(suite)))


@nottest
def get_failed_tests_message(results):
    has_failures = lambda suite: suite['failed'] != 0
    failed_suites = filter(has_failures, extract_dicts(results))
    return sum(map(list_failures, failed_suites), [])
-- 
https://code.launchpad.net/~allenap/maas/try-out-saucelabs/+merge/106217
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/try-out-saucelabs into lp:maas.