This is an archive of the discontinued LLVM Phabricator instance.

[clang] [python] [tests] Rewrite to use standard unittest module
ClosedPublic

Authored by mgorny on Nov 7 2017, 2:59 PM.

Details

Summary

Rewrite the tests from using plain 'assert' mixed with some nosetests
methods to the standard unittest module layout. Improve the code
to use the most canonical assertion methods whenever possible.

This has a few major advantages:

  • the code uses standard methods now, resulting in a reduced number of WTFs whenever someone with basic Python knowledge gets to read it,
  • completely unnecessary dependency on nosetests is removed since the standard library supplies all that is necessary for the tests to run,
  • the tests can be run via any test runner, including the one built-in in Python,
  • the failure output for most of the tests is improved from 'assertion x == y failed' to actually telling the values.

Diff Detail

Repository
rL LLVM

Event Timeline

mgorny created this revision.Nov 7 2017, 2:59 PM
mgorny edited the summary of this revision. (Show Details)
mgorny edited the summary of this revision. (Show Details)
mgorny added a comment.Nov 7 2017, 3:05 PM

The diffs aren't very useful since I had to add a class for each test unit and so everything needed reindenting. You can take my word that changes boil down to:

  • adding unittest import,
  • adding class for each test unit and converting the functions into methods (except for some generic utility funcs and vars which I left global scope),
  • replacing assertions and other checks with appropriate self.assert* methods (especially improving exception checks).

I get the same results (3 failures) with the change as I got before it.

mgorny added a comment.Nov 7 2017, 3:07 PM

Before:

======================================================================
FAIL: tests.cindex.test_code_completion.test_code_complete_availability
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib64/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/usr/src/llvm/tools/clang/bindings/python/tests/cindex/test_code_completion.py", line 75, in test_code_complete_availability
    check_completion_results(cr, expected)
  File "/usr/src/llvm/tools/clang/bindings/python/tests/cindex/test_code_completion.py", line 10, in check_completion_results
    assert c in completions
AssertionError

======================================================================
FAIL: Ensure that linkage specifers are available on cursors
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib64/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/usr/src/llvm/tools/clang/bindings/python/tests/cindex/test_linkage.py", line 26, in test_linkage
    assert unique_external.linkage == LinkageKind.UNIQUE_EXTERNAL
AssertionError

======================================================================
FAIL: Ensure that thread-local storage kinds are available on cursors.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib64/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/usr/src/llvm/tools/clang/bindings/python/tests/cindex/test_tls_kind.py", line 36, in test_tls_kind
    assert tls_declspec.tls_kind == TLSKind.STATIC
AssertionError

----------------------------------------------------------------------
Ran 117 tests in 1.460s

FAILED (failures=3)

After:

======================================================================
FAIL: test_code_complete_availability (tests.cindex.test_code_completion.TestCodeCompletion)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/src/llvm/tools/clang/bindings/python/tests/cindex/test_code_completion.py", line 79, in test_code_complete_availability
    self.check_completion_results(cr, expected)
  File "/usr/src/llvm/tools/clang/bindings/python/tests/cindex/test_code_completion.py", line 14, in check_completion_results
    self.assertIn(c, completions)
AssertionError: "{'P &', ResultType} | {'operator=', TypedText} | {'(', LeftParen} | {'const P &', Placeholder} | {')', RightParen} || Priority: 34 || Availability: Available || Brief comment: None" not found in ["{'void', ResultType} | {'~P', TypedText} | {'(', LeftParen} | {')', RightParen} || Priority: 79 || Availability: Available || Brief comment: None", "{'P &', ResultType} | {'operator=', TypedText} | {'(', LeftParen} | {'const P &', Placeholder} | {')', RightParen} || Priority: 79 || Availability: Available || Brief comment: None", "{'P', TypedText} | {'::', Text} || Priority: 75 || Availability: Available || Brief comment: None", "{'int', ResultType} | {'member', TypedText} || Priority: 35 || Availability: NotAccessible || Brief comment: None"]

======================================================================
FAIL: Ensure that linkage specifers are available on cursors
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/src/llvm/tools/clang/bindings/python/tests/cindex/test_linkage.py", line 29, in test_linkage
    self.assertEqual(unique_external.linkage, LinkageKind.UNIQUE_EXTERNAL)
AssertionError: LinkageKind.INTERNAL != LinkageKind.UNIQUE_EXTERNAL

======================================================================
FAIL: Ensure that thread-local storage kinds are available on cursors.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/src/llvm/tools/clang/bindings/python/tests/cindex/test_tls_kind.py", line 39, in test_tls_kind
    self.assertEqual(tls_declspec.tls_kind, TLSKind.STATIC)
AssertionError: TLSKind.DYNAMIC != TLSKind.STATIC

----------------------------------------------------------------------
Ran 117 tests in 1.471s

FAILED (failures=3)
bkramer accepted this revision.Nov 10 2017, 5:37 AM

I can't really check if all the tests are equivalent to the old ones, but not having to install nose is a major usability improvement. Let's ship this.

We should totally fix the existing failures. I assume this is because nobody ever runs the tests.

This revision is now accepted and ready to land.Nov 10 2017, 5:37 AM

Thanks. I'll update it to incorporate the lately committed test fixes, and push it later today.

This revision was automatically updated to reflect the committed changes.