On 12/05, Stefan Beller wrote: > On Mon, Dec 5, 2016 at 11:29 AM, Brandon Williams <bmwill@xxxxxxxxxx> wrote: > > On 12/02, Stefan Beller wrote: > >> +test_expect_success 'option checkout.recurseSubmodules updates submodule' ' > >> + test_config checkout.recurseSubmodules 1 && > >> + git checkout base && > >> + git checkout -b advanced-base && > >> + git -C submodule commit --allow-empty -m "empty commit" && > >> + git add submodule && > >> + git commit -m "advance submodule" && > >> + git checkout base && > >> + git diff-files --quiet && > >> + git diff-index --quiet --cached base && > >> + git checkout advanced-base && > >> + git diff-files --quiet && > >> + git diff-index --quiet --cached advanced-base && > >> + git checkout --recurse-submodules base > >> +' > >> + > > > > This test doesn't look like it looks into the submodule to see if the > > submodule has indeed changed. Unless diff-index and diff-files recurse > > into the submodules? > > I took the code from Jens once upon a time. Rereading the code, I agree it is > not obvious how this checks the submodule state. > > However `git diff-files --quiet` is perfectly fine, as > we have submodule support by default via: > > Omitting the --submodule option or specifying --submodule=short, > uses the short format. This format just shows the names of the commits > at the beginning and end of the range. > > and then we turn it into an exit code via > > --quiet > Disable all output of the program. Implies --exit-code. > > --exit-code > Make the program exit with codes similar to diff(1). > That is, it exits with 1 if there were differences and 0 means no differences. > > Same for diff-index. > > The main purpose of this specific test is to have checkout.recurseSubmodules > "to just make it work" without having to give --recurse-submodules manually. > All the other tests with the manual --recurse-submodules should test for > correctness of the behavior within the submodule. > > So maybe I'll need to rewrite submodule_creation_must_succeed() in the previous > patch to be more obvious. (Well that already has some tests for > files/directories > in there, so it is a little more.) > > But to be sure we can also add tests here that look more into the submodule. > I am thinking of "{new,old}_sub_sha1=$(git -C submodule rev-parse HEAD)" and > comparing them? Ah ok, that makes sense now. Its kind of like if I run git status it would show if a submodule is at a different sha1 than the superproject has recorded. -- Brandon Williams