On Mon, Nov 6, 2017 at 5:52 PM, Junio C Hamano <gits...@pobox.com> wrote: > Jonathan Tan <jonathanta...@google.com> writes: > >> Factor out the commonalities from test_submodule_switch() and >> test_submodule_forced_switch() in lib-submodule-update.sh, and document >> their usage. >> >> This also makes explicit (through the KNOWN_FAILURE_FORCED_SWITCH_TESTS >> variable) the fact that, currently, all functionality tested using >> test_submodule_forced_switch() do not correctly handle the situation in >> which a submodule is replaced with an ordinary directory. >> >> Signed-off-by: Jonathan Tan <jonathanta...@google.com> >> --- >> I find tests that use lib-submodule-update.sh difficult to understand >> due to the lack of clarity of what test_submodule_switch() and others do >> with their argument - I hope this will make things easier for future >> readers. >> --- >> t/lib-submodule-update.sh | 250 >> +++++++++------------------------------------- >> 1 file changed, 46 insertions(+), 204 deletions(-) > > I suspect that the benefit of this is a lot larger than "document" a > test helper function or two ;-) "document & clean-up", perhaps?
It is. > > I didn't compare the before-and-after with fine toothed comb, but > a cursory look didn't find anything glaringly questionable other > than the above. > I have reviewed the patch and thinks it withstands the test of a fine comb.