Bug 1612223 - Group Push Health crash signatures (#5985)

tests/push_health/test_tests.py

@@ -0,0 +1,37 @@
+import pytest
+
+from treeherder.model.models import (FailureLine,
+                                     Job,
+                                     Repository)
+from treeherder.push_health.tests import (has_job,
+                                          has_line)
+
+
+@pytest.mark.parametrize(('find_it',), [(True,), (False,)])
+def test_has_job(find_it):
+    job = Job(id=123, repository=Repository(), guid='12345')
+    job_list = [
+        {'id': 111},
+        {'id': 222},
+    ]
+
+    if find_it:
+        job_list.append({'id': 123})
+        assert has_job(job, job_list)
+    else:
+        assert not has_job(job, job_list)
+
+
+@pytest.mark.parametrize(('find_it',), [(True,), (False,)])
+def test_has_line(find_it):
+    line = FailureLine(line=123)
+    line_list = [
+        {'line_number': 111},
+        {'line_number': 222},
+    ]
+
+    if find_it:
+        line_list.append({'line_number': 123})
+        assert has_line(line, line_list)
+    else:
+        assert not has_line(line, line_list)

tests/push_health/test_utils.py

@@ -0,0 +1,61 @@
+import pytest
+
+from treeherder.push_health.utils import (clean_config,
+                                          clean_platform,
+                                          clean_test,
+                                          is_valid_failure_line)
+
+
+@pytest.mark.parametrize(('action', 'test', 'signature', 'message', 'expected'), [
+     ('test_result', 'dis/dat/da/odder/ting', 'sig', 'mess', 'dis/dat/da/odder/ting'),
+     ('crash', 'dis/dat/da/odder/ting', 'sig', 'mess', 'sig'),
+     ('log', 'dis/dat/da/odder/ting', 'sig', 'mess', 'mess'),
+     ('meh', 'dis/dat/da/odder/ting', 'sig', 'mess', 'Non-Test Error'),
+     ('test_result', 'pid:dis/dat/da/odder/ting', 'sig', 'mess', None),
+     ('test_result', 'tests/layout/this == tests/layout/that', 'sig', 'mess', 'layout/this == layout/that'),
+     ('test_result', 'tests/layout/this != tests/layout/that', 'sig', 'mess', 'layout/this != layout/that'),
+     ('test_result', 'build/tests/reftest/tests/this != build/tests/reftest/tests/that', 'sig', 'mess', 'this != that'),
+     ('test_result', 'http://10.0.5.5/tests/this != http://10.0.5.5/tests/that', 'sig', 'mess', 'this != that'),
+     ('test_result', 'build/tests/reftest/tests/this', 'sig', 'mess', 'this'),
+     ('test_result', 'test=jsreftest.html', 'sig', 'mess', 'jsreftest.html'),
+     ('test_result', 'http://10.0.5.5/tests/this/thing', 'sig', 'mess', 'this/thing'),
+     ('test_result', 'http://localhost:5000/tests/this/thing', 'sig', 'mess', 'thing'),
+     ('test_result', 'thing is done (finished)', 'sig', 'mess', 'thing is done'),
+     ('test_result', 'Last test finished', 'sig', 'mess', None),
+     ('test_result', '(SimpleTest/TestRunner.js)', 'sig', 'mess', None),
+     ('test_result', '/this\\thing\\there', 'sig', 'mess', 'this/thing/there'),
+    ])
+def test_clean_test(action, test, signature, message, expected):
+    assert expected == clean_test(action, test, signature, message)
+
+
+@pytest.mark.parametrize(('config', 'expected'), [
+     ('opt', 'opt'),
+     ('debug', 'debug'),
+     ('asan', 'asan'),
+     ('pgo', 'opt'),
+     ('shippable', 'opt'),
+    ])
+def test_clean_config(config, expected):
+    assert expected == clean_config(config)
+
+
+@pytest.mark.parametrize(('platform', 'expected'), [
+     ('macosx64 opt and such', 'osx-10-10 opt and such'),
+     ('linux doohickey', 'linux doohickey'),
+     ('windows gizmo', 'windows gizmo'),
+    ])
+def test_clean_platform(platform, expected):
+    assert expected == clean_platform(platform)
+
+
+@pytest.mark.parametrize(('line', 'expected'), [
+     ('Return code:', False),
+     ('unexpected status', False),
+     ('unexpected crashes', False),
+     ('exit status', False),
+     ('Finished in', False),
+     ('expect magic', True),
+  ])
+def test_is_valid_failure_line(line, expected):
+    assert expected == is_valid_failure_line(line)

tests/ui/push-health/GroupedTests_test.jsx

@@ -29,7 +29,7 @@ describe('GroupedTests', () => {
 
     expect(
       await waitForElement(() => getAllByTestId('test-grouping')),
-    ).toHaveLength(32);
+    ).toHaveLength(35);
   });
 
   test('should filter when grouped by test path', async () => {

tests/ui/push-health/TestFailure_test.jsx

@@ -15,7 +15,8 @@ import TestFailure from '../../../ui/push-health/TestFailure';
 import pushHealth from '../mock/push_health';
 
 const repoName = 'autoland';
-const failure = pushHealth.metrics.tests.details.needInvestigation[0];
+const crashFailure = pushHealth.metrics.tests.details.needInvestigation[0];
+const testFailure = pushHealth.metrics.tests.details.needInvestigation[2];
 const cssFile = fs.readFileSync(
   path.resolve(
     __dirname,
@@ -43,7 +44,7 @@ beforeEach(() => {
     },
   });
   setUrlParam('repo', repoName);
-  failure.key = 'wazzon';
+  testFailure.key = 'wazzon';
 });
 
 afterEach(() => {
@@ -66,7 +67,7 @@ describe('TestFailure', () => {
   );
 
   test('should show the test name', async () => {
-    const { getByText } = render(testTestFailure(failure));
+    const { getByText } = render(testTestFailure(testFailure));
 
     expect(
       await waitForElement(() =>
@@ -76,7 +77,7 @@ describe('TestFailure', () => {
   });
 
   test('should not show details by default', async () => {
-    const { container, getByText } = render(testTestFailure(failure));
+    const { container, getByText } = render(testTestFailure(testFailure));
     useStyles(container);
 
     // Must use .toBeVisible() rather than .toBeInTheDocument because
@@ -94,7 +95,7 @@ describe('TestFailure', () => {
   });
 
   test('should show details when click more...', async () => {
-    const { container, getByText } = render(testTestFailure(failure));
+    const { container, getByText } = render(testTestFailure(testFailure));
     const moreLink = getByText('more...');
 
     useStyles(container);
@@ -112,4 +113,28 @@ describe('TestFailure', () => {
       await waitForElement(() => getByText('less...')),
     ).toBeInTheDocument();
   });
+
+  test('should show crash stack and signature when click more...', async () => {
+    const { container, getByText, getAllByText } = render(
+      testTestFailure(crashFailure),
+    );
+    const moreLink = getByText('more...');
+
+    useStyles(container);
+    fireEvent.click(moreLink);
+
+    expect(
+      await waitForElement(
+        () => getAllByText('@ nsDebugImpl::Abort(char const*, int)')[0],
+      ),
+    ).toBeVisible();
+    expect(
+      await waitForElement(() =>
+        getByText('Operating system: Mac OS X', { exact: false }),
+      ),
+    ).toBeVisible();
+    expect(
+      await waitForElement(() => getByText('less...')),
+    ).toBeInTheDocument();
+  });
 });

treeherder/push_health/tests.py

@@ -50,14 +50,17 @@ def get_history(failure_classification_id, push_date, num_days, option_map, repo
         ).select_related(
             'job_log__job__machine_platform', 'job_log__job__push'
         ).values(
+            'action',
             'test',
+            'signature',
+            'message',
             'job_log__job__machine_platform__platform',
             'job_log__job__option_collection_hash'
         ).distinct()
         previous_failures = defaultdict(lambda: defaultdict(lambda: defaultdict(int)))
         for line in failure_lines:
             previous_failures[
-                clean_test(line['test'])
+                clean_test(line['action'], line['test'], line['signature'], line['message'])
             ][
                 clean_platform(line['job_log__job__machine_platform__platform'])
             ][
@@ -106,7 +109,9 @@ def get_current_test_failures(push, option_map):
     tests = {}
     all_failed_jobs = {}
     for failure_line in new_failure_lines:
-        test_name = clean_test(failure_line.test)
+        test_name = clean_test(
+            failure_line.action, failure_line.test, failure_line.signature, failure_line.message
+        )
         if not test_name:
             continue
         job = failure_line.job_log.job

treeherder/push_health/utils.py

@@ -1,6 +1,13 @@
-def clean_test(test_name):
+def clean_test(action, test, signature, message):
     try:
-        clean_name = str(test_name) if test_name else 'Non-Test Error'
+        clean_name = 'Non-Test Error'
+        if action == 'test_result':
+            clean_name = test
+        elif action == 'crash':
+            clean_name = signature
+        elif action == 'log':
+            clean_name = message if len(message) < 50 else '{}...'.format(message[:50])
+
     except UnicodeEncodeError:
         return ''
 
@@ -8,10 +15,8 @@ def clean_test(test_name):
         return None
 
     if ' == ' in clean_name or ' != ' in clean_name:
-        if ' != ' in clean_name:
-            left, right = clean_name.split(' != ')
-        elif ' == ' in clean_name:
-            left, right = clean_name.split(' == ')
+        splitter = ' == ' if ' == ' in clean_name else ' != '
+        left, right = clean_name.split(splitter)
 
         if 'tests/layout/' in left and 'tests/layout/' in right:
             left = 'layout%s' % left.split('tests/layout')[1]
@@ -23,7 +28,7 @@ def clean_test(test_name):
         elif clean_name.startswith('http://10.0'):
             left = '/tests/'.join(left.split('/tests/')[1:])
             right = '/tests/'.join(right.split('/tests/')[1:])
-        clean_name = "%s == %s" % (left, right)
+        clean_name = "%s%s%s" % (left, splitter, right)
 
     if 'build/tests/reftest/tests/' in clean_name:
         clean_name = clean_name.split('build/tests/reftest/tests/')[1]
@@ -44,8 +49,7 @@ def clean_test(test_name):
 
     # Now that we don't bail on a blank test_name, these filters
     # may sometimes apply.
-    if clean_name in [None,
-                      'Last test finished',
+    if clean_name in ['Last test finished',
                       '(SimpleTest/TestRunner.js)']:
         return None
 

ui/push-health/Health.jsx

@@ -60,7 +60,7 @@ export default class Health extends React.PureComponent {
     const expandedStates = Object.entries(metrics).reduce(
       (acc, [key, metric]) => ({
         ...acc,
-        [`${key}Expanded`]: metric.result !== 'pass',
+        [`${key}Expanded`]: metric.result === 'fail',
       }),
       {},
     );

ui/push-health/TestFailure.jsx

@@ -3,6 +3,7 @@ import PropTypes from 'prop-types';
 import {
   Badge,
   Button,
+  Container,
   Row,
   Col,
   UncontrolledTooltip,
@@ -160,18 +161,42 @@ class TestFailure extends React.PureComponent {
               </Button>
               <UncontrolledCollapse toggler={key}>
                 {logLines.map(logLine => (
-                  <Row
-                    className="small text-monospace mt-2 ml-3"
-                    key={logLine.line_number}
-                  >
-                    <div className="pre-wrap text-break">
+                  <Row className="small mt-2" key={logLine.line_number}>
+                    <Container className="pre-wrap text-break">
                       {logLine.subtest}
-                      <Row className="ml-3">
-                        <div>{logLine.message}</div>
-                        <div>{logLine.signature}</div>
-                        <div>{logLine.stackwalk_stdout}</div>
+                      <Col>
+                        {logLine.message && (
+                          <Row className="mb-3">
+                            <Col xs="1" className="font-weight-bold">
+                              Message:
+                            </Col>
+                            <Col className="text-monospace">
+                              {logLine.message}
+                            </Col>
                           </Row>
-                    </div>
+                        )}
+                        {logLine.signature && (
+                          <Row className="mb-3">
+                            <Col xs="1" className="font-weight-bold">
+                              Signature:
+                            </Col>
+                            <Col className="text-monospace">
+                              {logLine.signature}
+                            </Col>
+                          </Row>
+                        )}
+                        {logLine.stackwalk_stdout && (
+                          <Row className="mb-3">
+                            <Col xs="1" className="font-weight-bold">
+                              Stack
+                            </Col>
+                            <Col className="text-monospace">
+                              {logLine.stackwalk_stdout}
+                            </Col>
+                          </Row>
+                        )}
+                      </Col>
+                    </Container>
                   </Row>
                 ))}
               </UncontrolledCollapse>