Stefan Beller <sbeller@xxxxxxxxxx> writes: > On Mon, Nov 21, 2016 at 11:07 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: >> Stefan Beller <sbeller@xxxxxxxxxx> writes: >> >>> So I guess we should test a bit more extensively, maybe >>> >>> git status >expect >>> git submodule embedgitdirs >>> git status >actual >>> test_cmp expect actual >>> # further testing via >>> test -f .. >>> test -d .. >> >> Something along that line. "status should succeed" does not tell >> the readers what kind of breakage the test is expecting to protect >> us from. If we are expecting a breakage in embed-git-dirs would >> somehow corrupt an existing submodule, which would lead to "status" >> that is run in the superproject report the submodule differently, >> then comparing output before and after the operation may be a >> reasonable test. Going there to the submodule working tree and >> checking the health of the repository (of the submodule) may be >> another sensible test. > > and by checking the health you mean just a status in there, or rather a > more nuanced thing like `git rev-parse HEAD` ? I'll go with that for now. Would fsck have caught the breakages you caused while developing it, for example? As the core of the "embed" operation is an non-atomic "move the .git directory to .git/modules/$name in the superproject and then make a gitfile pointing at it", verifying that there is no change in the contents of .git/modules before and after the failed operation, and verifying that the submodule working tree has the embedded .git/ directory would be good enough, I would think.