From: Felipe Contreras Date: Thu, 18 Apr 2013 13:06:00 GMT Subject: Re: [PATCH 1/6] transport-helper: clarify *:* refspec Message-ID: In-Reply-To: <87ppxsvwq7.fsf@linux-k42r.v.cablecom.net> On Thu, Apr 18, 2013 at 7:39 AM, Thomas Rast wrote: > Felipe Contreras writes: > >> On Thu, Apr 18, 2013 at 2:28 AM, Thomas Rast wrote: >>> Felipe Contreras writes: >>> >>>> The *:* refspec doesn't work, and never has, clarify the code and >>>> documentation to reflect that. This in effect reverts commit 9e7673e >>>> (gitremote-helpers(1): clarify refspec behaviour). >>> [...] >>>> -test_expect_success 'pulling with straight refspec' ' >>>> - (cd local2 && >>>> - GIT_REMOTE_TESTGIT_REFSPEC="*:*" git pull) && >>>> - compare_refs local2 HEAD server HEAD >>>> -' >>>> - >>>> -test_expect_failure 'pushing with straight refspec' ' >>>> - test_when_finished "(cd local2 && git reset --hard origin)" && >>>> - (cd local2 && >>>> - echo content >>file && >>>> - git commit -a -m eleven && >>>> - GIT_REMOTE_TESTGIT_REFSPEC="*:*" git push) && >>>> - compare_refs local2 HEAD server HEAD >>>> -' >>> >>> So what's wrong with the tests? Do they fail to test what they claim >>> (how?), test something that wasn't reasonable to begin with, or >>> something entirely different? >> >> Look at the code comment, and look at the now updated documentation >> that assumes that *:* was reasonable. Given the available information, >> it would be reasonable to assume that *:* did work, but it didn't >> work, and it's not really possible to fix it, even if we wanted to, it >> would be a hack. It's better to accept that fact and stop worrying too >> much about what would be the best way to do the wrong thing. > > Ok, you say that the *failing* test set an expectation that is > unrealistic, so let's drop it. > > But then what about the successful test? Does it actually work (and by > removing the test, you are saying that we don't care if we subsequently > break that (mis)feature)? Or did it test the wrong thing? Yeah, it works, in the sense that peeing in a bottle is a solution; it might work, but it's not recommendable. So, if suddenly working, frankly I don't care. I added those tests, and I don't think they are needed. In a not too distant future it should not be permitted to "work"; we don't want developers to shoot themselves in the foot, and heir users too. -- Felipe Contreras