Refactor p4 invocation from PerforceClient, and add unit tests.
Review Request #3710 — Created Jan. 3, 2013 and submitted
Refactor p4 invocation from PerforceClient, and add unit tests. In an effort to clean up PerforceClient a little and make it testable, this change consolidates all the p4 invocation into a class called P4Wrapper. A P4Wrapper class is passed to PerforceClient (defaults to P4Wrapper, but unit tests can override this), and an instance will be created from this. Callers can call into this and just deal with the results, regardless of how p4 is called. This allowed some unit tests to be written. While this is by no means comprehensive, it does add some basic testing for things like old-style and new-style Perforce counters, get_repository_info, and basic diff building. I'll be making use of these new tests for Perforce moved file support. Going forward, we probably should look into using p4 -G to get marshalled Python objects as results from the commands (in order to avoid doing our own parsing), and add some exceptions for failure so that PerforceClient doesn't have to parse error strings.
Unit tests pass. I played around with post-review a little with a Perforce client and didn't see any regressions, but further, more real-world testing would be helpful.
Description | From | Last Updated |
---|---|---|
Is there a reason you wanted to keep this p4 invocation separate? I know it's simple, but we're going against … |
SM smacleod | |
Remove this blank line. |
SM smacleod | |
Col: 80 E501 line too long (81 > 79 characters) |
reviewbot | |
Col: 80 E501 line too long (97 > 79 characters) |
reviewbot | |
Col: 13 E128 continuation line under-indented for visual indent |
reviewbot |