python-test-framework icon indicating copy to clipboard operation
python-test-framework copied to clipboard

RFC: Use `AssertionError` when inside decorated test case

Open kazk opened this issue 5 years ago • 7 comments

  • Stops at first failure like most other test frameworks
  • Single test passed message regardless of the number of assertions
  • Discourage fat test cases
  • Allow using external assertions
    • np.testing.assert_equal
    • pd.testing.assert_frame_equal
    • assertion packages

Tests written in old style are not affected.

Need some review and feedback.


Closes #9

kazk avatar Jul 13 '20 04:07 kazk

Need some review and feedback

Are you interested in feedback on the design, code/style, both, or some other aspect(s)?

ggorlen avatar Aug 06 '20 19:08 ggorlen

Any feedback. But I'm honestly starting to think it's better to spend time on figuring out how to allow kata authors to choose unittest instead.

kazk avatar Aug 06 '20 19:08 kazk

I'm curious what the rationale is behind limiting the AssertionError catch to the decorated test case function only. Yeah, unittest support would be great (if possible)!

ggorlen avatar Sep 20 '20 20:09 ggorlen

I'm curious what the rationale is behind limiting the AssertionError catch to the decorated test case function only.

I'm not sure what you mean by catch. Can you post a link to the line or leave a comment on the line?


So old tests not using decorators looks like this:

test.it("old test")
test.assert_equals(1, 2)

If this assert_equals raised AssertionError, we won't get any failure message (i.e., no <FAILED::>) and test just crashes there.

kazk avatar Sep 21 '20 03:09 kazk

Maybe "handle" is a better term. No particular line--the design seems based on # TODO Currently this only works if assertion functions are written directly in the test case., but wouldn't it be possible to try-except the entire decorated test function execution, catch (or except) any AssertionErrors that are raised (regardless of whether they were raised in the test case function or not) and print the necessary <FAILED::> along with whatever error message rode along the error object?

ggorlen avatar Sep 21 '20 04:09 ggorlen

That comment means

import codewars_test as test
@test.describe("group 1")
def group_1():
    @test.it("test 1")
    def test_1():
        test.assert_equals(1, 2)

works, but

import codewars_test as test

def wrapped_equal(a, b):
    test.assert_equal(a, b)

@test.describe("group 1")
def group_1():
    @test.it("test 1")
    def test_1():
        wrapped_equal(1, 2)

doesn't because of the way it currently checks if it's inside a test case. We can make it work better, but I didn't think it's worth it for RFC.

wouldn't it be possible to try-except the entire decorated test function execution

We can do that if we didn't have to worry about the existing tests using assertions from this package (test.assert_equal) on top level. We can't change them to always throw AssertionError because that breaks a lot. And if it doesn't throw, we can't report the failure. That's why changes in this PR only changes their behavior when inside a test case.

kazk avatar Sep 21 '20 17:09 kazk

Also, to be clear, that limitation only applies for assertions from this package. Anything that throws AssertionError should work even if it's deeply nested.

kazk avatar Sep 21 '20 17:09 kazk