Yelp / Testify

A more pythonic testing framework.
Other
304 stars 67 forks source link

testify doesn't include class_setup/teardown in "Total test time" #18

Open coyotemarin opened 13 years ago

coyotemarin commented 13 years ago

A lot of time spent in Yelp tests is not the actual tests, but setting up the users/reviews/etc. to run the tests on. Currently Testify doesn't show timing information for @class_setup and @class_teardown methods, or include them in the "Total test time".

These tend to be the most expensive methods (otherwise we'd just use @setup or @teardown), so it's kind of a liability not to know just how expensive they are.

rhettg commented 13 years ago

Where would you like to see this time reported?

coyotemarin commented 13 years ago

I'd expect "Total test time" to include class setup and teardown.

In verbose mode, I'd expect to see a line like:

path.to.test_module SlowAssTestCase.do_expensive_stuff ... class_setup in 121.46s
rhettg commented 13 years ago

Maybe if we listed the class setup time like:

path.to.test_module SlowAssTestCase.class_setup ... 121.46s
path.to.test_module SlowAssTestCase.test ... 2.1s
path.to.test_module SlowAssTestCase.class_teardown ... 5.34s

I think I didn't understand what you meant by "Total test time". If that's truely just the sum of all the reported test method times then that definitely sounds like a bug.

coyotemarin commented 13 years ago

Yup, that's exactly the format I'd like. :)

Yeah, it looks like class setup/teardown times are not included in total test time.

coyotemarin commented 13 years ago

Oh, except that you should still say ok after tests that succeed. But yeah, it's fine to have all class setup in a single line; I imagine it could get quite cluttered otherwise.

Something like:

path.to.test_module SlowAssTestCase ... class_setup in 121.46s

might be a little better, since it doesn't make class_setup look like a method name.

eskil commented 12 years ago

Oh yeah, just git confused by this as well ; 6s test taking almost 3min.

[sandbox] eskil@dev19$ time testify -x disabled yelp.tests.cmds.category_browse_test 
./yelp/testing/testutil/generators.py:95: UnicodeWarning: Unicode equal comparison failed to convert both arguments to Unicode - interpreting them as being unequal
  for c in set(r) - allowed_chars:
..................
PASSED.  18 tests / 2 cases: 18 passed, 0 failed.  (Total test time 6.73s)

real    2m41.232s
user    1m39.810s
sys 0m9.630s
EvanKrall commented 12 years ago

I've got a branch that causes class_setup/class_teardown to get reported to the reporting plugins. (https://github.com/Yelp/Testify/pull/75)

I suppose I could go ahead and make the default reporter spit out this info.

mrtyler commented 11 years ago

116 adds callback hooks for class_setup and class_teardown time. (This code is cherry picked from one of @EvanKrall's branches, possibly #75). That should make it quite a bit easier to implement the requested feature.