Jeff King <peff@xxxxxxxx> writes: > You solve it here by passing OVERWRITING_FAIL down into the callback > functions. And that does work. But I think it may be easier to > understand if we invert the responsibility. Let the outer caller specify > two callbacks: one for setup/prep that must succeed, and one for a > single operation where we might expect success or failure. > > The changes in lib-submodule-update look something like: > ... > And in the caller we can simplify greatly: > ... > +test_submodule_switch "reset_branch_to_HEAD" "git pull" > +test_submodule_switch "reset_branch_to_HEAD" "git pull -ff" > +test_submodule_switch "reset_branch_to_HEAD" "git pull --ff-only" > KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1 > KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1 > -test_submodule_switch "git_pull_noff" > +test_submodule_switch "reset_branch_to_HEAD" "git pull --no-ff" > > test_expect_success 'pull --recurse-submodule setup' ' > test_create_repo child && > > I suspect this approach involves touching more lines than yours (it has > to add $prep everywhere $command is used). But IMHO the result is much > simpler to understand, because there's no spooky-action-at-a-distance > from magic environment variables. Yes, spooky-action-at-a-distance was what made me go Eeeeek when I saw the original approach. > A more complete patch is below, which is enough to get t5572 passing. I > think it would need the other test_submodule_switch() function updated, > and other scripts would need to adapt to the 2-arg style. below where?
