Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
coyotemarin opened this issue Jun 8, 2011 · 8 comments
Open

Comments

@coyotemarin
Copy link

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
Copy link
Contributor

rhettg commented Jun 8, 2011

Where would you like to see this time reported?

@coyotemarin
Copy link
Author

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
Copy link
Contributor

rhettg commented Jun 8, 2011

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
Copy link
Author

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
Copy link
Author

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
Copy link
Contributor

eskil commented Feb 29, 2012

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
Copy link
Member

I've got a branch that causes class_setup/class_teardown to get reported to the reporting plugins. (#75)

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

@mrtyler
Copy link
Contributor

mrtyler commented Dec 7, 2012

#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants