On 2012-6-7 21:26, Adam Litke wrote:
On Thu, Jun 07, 2012 at 12:03:30PM +0800, Zhou Zheng Sheng wrote:
Hi,

Since there is no coverage report on tests in vdsm, if a PEP 8 patch
passes the tests, we still not sure if there is no mistake in it.
Viewing the diff reports on all the changes consumes a lot of time, and
some small but fatal mistakes(like misspelling variable name) can easily
be ignored by human eyes.

So I have a try on the compiler module of Python. I write a tool named
'pydiff'. pydiff parses two Python scripts into Abstract Syntax Trees.
These data structures can reflect the logic of the code, and pydiff
performs a recursive compare on the trees. Then pydiff reports
differences and the corresponding line numbers. In this way, pydiff
ignores code style changes, and reports only logical changes of the code.

I think this tool can save us a lot of time. After a PEP 8 patch passes
vdsm tests and pydiff, I will get some confidence on the patch and it
probably does not break anything in vdsm.
This is a very nice tool.  Thanks for sharing it.  I would like to see all
authors of PEP8 patches use this to check their patches for semantic
correctness.  This should greatly improve our ability to complete the PEP8
cleanup quickly.

Yes, I agree with you. Also, we should merge this tool into vdsm as a helper for PEP8 clean work.



Here is a usage example:

-------- test_o.py --------
def foo(a, b):
pass

if __name__ == '__main__':
A = [1, 2, 3]
print (4, 5, 6), \
"over"
foo(1, 2)
print 'Hello World'


-------- test_n.py --------
def foo(a, b):
pass

if __name__ == '__main__':
A = [1,
2, 3]
print (4, 5, 6), "over"
fooo(
1, 2)
print ('Hello '
'World')


Some differences of the files are just a matter of style. The only
significant difference is the function call "foo()" is misspelled in
"test_n.py".

Run pydiff.py, it will report:

$ python pydiff.py test_*.py
1 difference(s)
first file: test_n.py
second file: test_o.py

((8, 'fooo'), (8, 'foo'))

This report tells us that 'fooo' in line 8 of "test_n.py" is different
from 'foo' in line 8 of "test_o.py".


It can also find insertions or deletions. Here is another simple example:

---- old.py ----
print 'Hello 1'
print 'Hello 2'
print 'Hello 3'
print 'Hello 4'
print 'Hello 5'

---- new.py ----
print 'Hello 1'
print 'Hello 3'
print 'Hello 4'
print 'Hello 5'
print 'Hello 5'

Run pydiff:

$ pydiff old.py new.py
2 difference(s)
first file: old.py
second file: new.py

((2, Printnl([Const('Hello 2')], None)), (2, None))

((5, None), (5, Printnl([Const('Hello 5')], None)))

Here "((2, Printnl([Const('Hello 2')], None)), (2, None))" means there
is a print statement in line 2 of old.py, but no corresponding statement
in new.py, so we can know the statement is deleted in new.py.
"((5, None), (5, Printnl([Const('Hello 5')], None)))" means there is a
print statement in line 5 of new.py, but no corresponding statement in
old.py, so we can know the statement is inserted in new.py.


Sometimes the change in code logic is acceptable, for example, change
"aDict.has_key(Key)" into "Key in aDict". pydiff can report a difference
in this case, but it is up to the user to judge whether it's acceptable.
pydiff is just a tool to help you finding these changes.

I hope it can be helpful for PEP 8 patch reviewers. If you find any
bugs, please let me know. The script is in the attachment.

--
Thanks and best regards!

Zhou Zheng Sheng / 周征晟
E-mail: zhshz...@linux.vnet.ibm.com
Telephone: 86-10-82454397


_______________________________________________
vdsm-devel mailing list
vdsm-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-devel



--
Shu Ming<shum...@linux.vnet.ibm.com>
IBM China Systems and Technology Laboratory


_______________________________________________
vdsm-devel mailing list
vdsm-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-devel

Reply via email to