[MERGE] push handles file-ids containing quotes correctly

[John Arbash Meinel]

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

Aaron Bentley wrote:
> John Arbash Meinel wrote:
>>> +1 on having our own _unescape that does the right thing. 
> 
> 
>>> +0 on having
>>> the tests all the way up at 'push'.
> 
> There are two tests.  One to make sure the push command does the right
> thing for evermore, and one to make sure that fetch does the right thing
> with all InterRepository implementations.
> 

...

> I'm just fixing a broken function, not rewriting it the way I think it
> should have been written in the first place.

Why not fix it the right way... Either way you are taking wrong behavior
and making it right.

...
> 
> If there is a codepath that can produce this, there is a programming
> error somewhere, so I think an assert is reasonable.

I don't really think of it as a codepath problem, but more of
unsanitized data. (It isn't like we are passing a int when we expect a
string, you are expected to pass data that you read in from a file).

> 
>>> It also means a single pass over the
>>> data rather than multiple passes.
> 
> This seems nicer to me, but I have no benchmarks to prove that the
> difference is significant.
> 
>>> Also, this should be tested directly (by calling _unescape_xml directly
>>> with several different inputs). If there is a problem higher up, then
>>> maybe we should also have a test for 'push'. But to start with, we
>>> should have direct tests of '_unescape_xml'.
> 
> Strongly disagree.  Private methods need not be tested, but their
> clients must be tested.
> 
> Aaron

Well, that is an argument for testing 'file_ids_altered_by_revision_ids'
not for testing 'push'.

I think it is fine to directly test a function so that it conforms to
its interface. We have lots of tests that directly call a private function.

We just have more flexibility in the future if we want to modify a
private function. It doesn't *have* to conform to the interface we gave
it. But test should fail so that we know we modified it somehow.

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

iD8DBQFEsoe3JdeBCYSNAAMRAsj4AKCezIw9auvFFom2Ovg5MtG6p6/2tgCgnv05
dPs9hSyTXzfMQfZU93vaAN0=
=6S+0
-----END PGP SIGNATURE-----