Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tests with latest go cmp #158

Merged
merged 3 commits into from
Aug 7, 2019

Conversation

dnephin
Copy link
Member

@dnephin dnephin commented Jun 25, 2019

Fixes #155

@thaJeztah
Copy link
Contributor

Looks like tests are failing (on older Go versions?) @dnephin
Wondering if those versions should still be tested though, because Go 1.10 and older reached EOL

@dnephin
Copy link
Member Author

dnephin commented Aug 5, 2019

Ya, I'd like to find a way to test it that works with all versions

@anthonyfok
Copy link
Contributor

anthonyfok commented Aug 6, 2019

I ran into this test error randomly, even with the PR patch applied:

=== RUN   TestDeepEqual
--- FAIL: TestDeepEqual (0.00s)
    compare_test.go:20: expected 
        "\n--- result\n+++ exp\n  []string{\n- \t\"a\",\n+ \t\"b\",\n- \t\"b\",\n+ \t\"a\",\n  }\n"
        got
        "\n--- result\n+++ exp\n\u00a0\u00a0[]string{\n-\u00a0\t\"a\",\n+\u00a0\t\"b\",\n-\u00a0\t\"b\",\n+\u00a0\t\"a\",\n\u00a0\u00a0}\n"

It turns out that go-cmp 0.3.0, the report randomly chooses between normal space (0x20) and non-breaking space (0xa0) in the report... I just got bitten by this in Debian packaging, as one build succeeds and yet the next build fails even though I did not change anything.

See google/go-cmp#124 for the whole story.

assert/cmp/compare_test.go Outdated Show resolved Hide resolved
Change the expected value for a couple of them to be less strict
@dnephin dnephin force-pushed the fix-tests-with-latest-go-cmp branch from ca155ff to 85ad333 Compare August 7, 2019 12:55
@codecov
Copy link

codecov bot commented Aug 7, 2019

Codecov Report

Merging #158 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #158   +/-   ##
=======================================
  Coverage   83.16%   83.16%           
=======================================
  Files          29       29           
  Lines        2038     2038           
=======================================
  Hits         1695     1695           
  Misses        236      236           
  Partials      107      107
Impacted Files Coverage Δ
internal/source/source.go 73.25% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 400e4d0...41b481e. Read the comment docs.

@dnephin dnephin force-pushed the fix-tests-with-latest-go-cmp branch 2 times, most recently from e852c27 to 0c37552 Compare August 7, 2019 13:20
It is not clear to me why this behaviour changed in go-cmp 0.3, but it seems to behave
differently in newer go versions
@dnephin dnephin force-pushed the fix-tests-with-latest-go-cmp branch from 0c37552 to 41b481e Compare August 7, 2019 13:21
@dnephin
Copy link
Member Author

dnephin commented Aug 7, 2019

Alright, tests seem to be passing now

@dnephin dnephin merged commit 04b215e into gotestyourself:master Aug 7, 2019
@dnephin dnephin deleted the fix-tests-with-latest-go-cmp branch August 7, 2019 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TestDeepEqualFailure fails w/ go 1.12.2
4 participants