[PATCH] Improve profiling doc and make the data easier to visualise

John Arbash Meinel john at arbash-meinel.com
Wed Jun 6 20:19:15 BST 2007


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ian Trustworthy wrote:
> John Arbash Meinel wrote:
> 
>> First, I think Robert was saying that there should be a unit-testable
>> helper function for determining the output format. And that it should
>> have unit tests. Not that it was something external that you need to run
>> after the profile has been run.
>>
>> Which I fully support.

...

> 
>> Other than that, +1 from me.
> 
> New bundle attached now. It probably doesn't need a full review again
> but I'd appreciate it if you could confirm that the saving helper method
> is as expected and unit tested sufficiently.
> 
> Thanks,
> Ian C.
> 

Well, I'm concerned about this line when _lsprof isn't installed:
=== added file bzrlib/tests/test_lsprof.py // file-id:test_lsprof.py-2007060609

...
+import bzrlib
+from bzrlib.lsprof import profile
+from bzrlib.tests import TestCaseInTempDir
+

I *think* doing "import bzrlib.lsprof" will fail if _lsprof isn't installed,
and that will cause the test suite to not load.

So I would prefer putting this in as a "Feature" and making sure lsprof is
loaded late, rather than at import test_lsprof time.


I would have thought that you would test this sort of thing doing something like:

self.assertEqual(PICKLE, file_type_for('test'))
self.assertEqual(PICKLE, file_type_for('test.pkl'))
self.assertEqual(TEXT, file_type_for('test.txt'))
self.assertEqual(PICKLE, file_type_for('testtxt'))

...

but what you have done is fine with me.

So if you fix it so that when _lsprof is not installed the test suite still
runs, +1 from me.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGZwizJdeBCYSNAAMRAhbXAJ9u1GqiR9RbOVRHcdUTNGt/bm3Z+wCeJEsb
0N2AS2CbkpBBTuDcGvJBliA=
=mQ4h
-----END PGP SIGNATURE-----



More information about the bazaar mailing list