gotest.tools golden: CRLF normalization conflicts with git core.autocrlf

👋 Thank you for working on golden ❤️

We are hitting some unexpected behaviour using golden on Windows. Strings are not being compared as I would expect. Here is some example output:

--- FAIL: TestEnvironmentSummary_golden_with_no_extra_settings (0.01s)
    parser_test.go:382: assertion failed: 
        --- expected
        +++ actual
        @@ -1,15 +1,15 @@
        -Using·build·environment·variables↵
        -··BASH_ENV=/tmp/.bash_env-qwerasdf↵
        -··CI=true↵
        -··CIRCLECI=true↵
        -··CIRCLE_BRANCH=↵
        -··CIRCLE_BUILD_NUM=321↵
        -··CIRCLE_BUILD_TOKEN=is-circle-build-token-still-a-thing?↵
        -··CIRCLE_JOB=↵
        -··CIRCLE_NODE_INDEX=0↵
        -··CIRCLE_NODE_TOTAL=0↵
        -··CIRCLE_REPOSITORY_URL=https://github.com/sample/example↵
        -··CIRCLE_SHA1=↵
        -··CIRCLE_SHELL_ENV=/tmp/.bash_env-qwerasdf↵
        -··CIRCLE_WORKSPACE_ID=myworkspace↵
        +Using·build·environment·variables
        +··BASH_ENV=/tmp/.bash_env-qwerasdf
        +··CI=true
        +··CIRCLECI=true
        +··CIRCLE_BRANCH=
        +··CIRCLE_BUILD_NUM=321
        +··CIRCLE_BUILD_TOKEN=is-circle-build-token-still-a-thing?
        +··CIRCLE_JOB=
        +··CIRCLE_NODE_INDEX=0
        +··CIRCLE_NODE_TOTAL=0
        +··CIRCLE_REPOSITORY_URL=https://github.com/sample/example
        +··CIRCLE_SHA1=
        +··CIRCLE_SHELL_ENV=/tmp/.bash_env-qwerasdf
        +··CIRCLE_WORKSPACE_ID=myworkspace

What I think is happening is that, on Windows, git will by default, automatically convert newline characters to CRLF on checkout (core.autocrlf).

So although golden converts the actual string to remove CR characters, it does not strip the CR characters from the expected string.

Mar 19 '19 21:03 marcomorain

@vdemeester @silvin-lubecki You've both contributed windows fixes for this type of thing. Have you ever run into this problem? Do you turn core.autocrlf off?

Mar 20 '19 01:03 dnephin

As fas as I remember, I fixed a similar issue with #105 I added a new PathOp MatchContentIgnoreCarriageReturn, you have to add explicitly to your tests so it can ignore windows carriage returns. But I don't think it applies with golden.

If I read the code correctly, the normalize function is only used while updating the golden file, and not for comparison.

Maybe we should introduce some PathOp options equivalent for the golden library? WDYT @dnephin ?

Mar 20 '19 09:03 silvin-lubecki

If I read the code correctly, the normalize function is only used while updating the golden file, and not for comparison.

removeCarriageReturn() is also called on the actual value before calling update. Looking at it again, it seems we don't need to pass it to update at all.

I think the idea is that we should always store the golden file with \n line endings. On windows the actual may contain \r\n so we remove them before comparing against the expected, and before we save the value to a file.

The problem in this issue is that git is changing the golden file to contain \r\n which is causing the assertion to fail because we attempt to normalize the comparison to always use \n.

Maybe we should introduce some PathOp options equivalent for the golden library?

I think a better option would be to remove the removeCarriageReturn() entirely if git is going to handle the normalization for us. Do you use the autocrlf git setting on windows?

Mar 20 '19 22:03 dnephin

Do you use the autocrlf git setting on windows?

Is the question for me? 🤔 On docker/app we use:

* text eol=lf

Mar 20 '19 23:03 silvin-lubecki

Thanks @silvin-lubecki

@marcomorain: maybe we set that value in .gitattributes for now?

I've opened a proposal for a change in #149, but the gitattributes change is likely a faster option.

Mar 20 '19 23:03 dnephin