Re: [PATCH] t2017: redo physical reflog existance check
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:49:22
Hi Erick, First, thanks for checkout --orphan. I was a skeptic but now that I have seen it being used for things similar to "going open source" (and the related "simplifying logs while working privately on a patch series") it looks to be a pretty nice tool. Erick Mattos wrote:
I don't see a need for so much reluctance: "test -f" is not a taboo inside a script in t folder and the added tests don't change anything about the design and implementation which IMHO is well fit.
The principle (though we do not always adhere to it) is that test
scripts should pass or fail based only on advertised behavior, not
implementation details. That way, _later_ any person who wants to
improve the implementation will not be impeded by tests.
The behavior that "test -f .git/logs/refs/heads/eta" checks for is not
part of the advertised behavior and though it does affect the
observable behavior, it is not immediately obvious how. Wouldn't it
be best if the test described that advertised behavior while checking
for it?
e.g.:
git config core.logallrefupdates false &&
test_when_finished "git config core.logallrefupdates true" &&
git checkout master &&
git checkout -l --orphan eta &&
test_must_fail git rev-parse --verify eta@{0} &&
test_tick &&
git commit -m "initial commit" &&
git rev-parse --verify eta@{0}
Happily, I am not the man in charge, so feel free to take my words
at whatever value you choose. :)
Regards,
Jonathan