[PATCH] bundle2: Add tests for multiple changegroup parts

[Pierre-Yves David]

On 09/01/2014 02:52 PM, Mike Hommey wrote:
> # HG changeset patch
> # User Mike Hommey <mh at glandium.org>
> # Date 1409575413 -32400
> # Node ID 34de1241da516e7896394893e66c7ac8ed9f9787
> # Parent  89382a0ed196d2923f1f0a6b7469fc23036a383c
> bundle2: Add tests for multiple changegroup parts

(You got the capitalization wrong, should be "bundle2: add tests". 
consider re-reading the "contributing changes" page on the wiki to 
clarify the test and runs the tests before patchbombing patch 
(`test-check-commit-hg.t` would have caught it))


Thanks for the effort, but adding such tests in that forms is not relevant:

1. That a bundle2 can contains the same part twice. This is already 
tested with a simpler part. We are not interested in double testing this 
with a complicated part that clobber the output.

2. You are mostly testing the output binary stream generated by a 
changegroup part. This is not the king of thing that are worthy to 
tests. The existing test check the binary output of the bundle2 for 
multiple kind of part to make sure that the bundle2 container is 
properly generated, but the actual content of part does not matters.

   You should focus of actual functional testing of the part in push and 
pull that would decides to generate multiple part or to process the 
result of this unbundling back to the user.

3. The part where you test the unbundling of a bundle2 with multiple 
changegroup is almost doing such functional testing, but it uses a 
simplistic and hacky command created in the test for its own purpose 
(and therefor are not relevant to real world usage)

To conclude, you should focus on testings the behavior of push and pull 
when there is multiple changegroup per bundle (could(should?) be 
artificially in a small extension created for the tests). And you can 
stick to the high level behavior of those function, no need to dig into 
the actual binary stream.

As you are opening a new chapter of the bundle2 history, it makes 
probably sense to start a new `test-bundle2-<something>.t` file


-- 
Pierre-Yves David