Re: [PATCH v2 3/7] Fix tests to work with core.autocrlf=true -- new functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/14/2009 3:43 AM, Charles Bailey wrote,

On Wed, May 13, 2009 at 03:35:44PM -0400, Don Slutz wrote:
Have I missed some previous recent discussion about this patch series?
I know that you referenced that long Aug 2007 thread about autocrlf,
but is there some more recent discussion about how the test suite
works / should work?

Not that I have seen. I did forget to send the previous reply, should now be there.
mergetool isn't the prime implementor of autocrlf, but it does have
some checks to make sure that it works with autocrlf. My impression -
probably incorrect - has been that autocrlf is off for the purposes of
building and testing git on all platforms, but that some packages
switch it on by default on install for user convenience on platforms
where this is appropriate.

Your patch seems to be about allowing the entire test suite to run
correctly with the autocrlf in any setting. If this is the case,
shouldn't the correct fix be to remove tests that are testing that
things work with different settings of autocrlf, because these tests
are effectively run by a full test suite run with autocrlf
alternatively set anyway?

Well my take is that most of the tests do not care about autocrlf, they are checking that the right file or error is happening. For example:


  git commit -m "c1"
 echo foo >file2
 git checkout -- file2

just wants to know that file2 is correctly reverted to before the change. The fact that file2 can have LF -> CRLF if autocrlf=true is not what is being checked for here. There are several tests like t0020-crlf.sh that are checking that the right thing happens.

I am assuming that t7610-mergetool.sh add the test for autocrlf=true because of some issue (bug?) that was fixed. However I have not done a full look into way the test is there. I was focused on getting the tests to pass.

Also I do expect that most people will run the tests in the default mode only. I have no plans on add the "run the tests in all possible settings of autocrlf".

This set seems like a subset (but still big) change on the path of getting git to work and pass the tests under CYGWIN on a text mount. That change is still in progress and not yet ready for the list.

  -Don


__________________________________________________________________________________________________________________
DISCLAIMER:"The information contained in this message and the attachments (if any) may be privileged and confidential and protected from disclosure. You are hereby notified that any unauthorized use, dissemination, distribution or copying of this communication, review, retransmission, or taking of any action based upon this information, by persons or entities other than the intended recipient, is strictly prohibited. If you are not the intended recipient or an employee or agent responsible for delivering this message, and have received this communication in error, please notify us immediately by replying to the message and kindly delete the original message, attachments, if any, and all its copies from your computer system. Thank you for your cooperation." ________________________________________________________________________________________________________________
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]