On Mon, Nov 6, 2017 at 5:52 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Jonathan Tan <jonathantanmy@xxxxxxxxxx> 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 <jonathantanmy@xxxxxxxxxx> >> --- >> 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.