Skip to content

Address expected vs actual in integration tests#36490

Merged
yongtang merged 1 commit into
moby:masterfrom
yongtang:03052018-expected-actual
Mar 6, 2018
Merged

Address expected vs actual in integration tests#36490
yongtang merged 1 commit into
moby:masterfrom
yongtang:03052018-expected-actual

Conversation

@yongtang
Copy link
Copy Markdown
Member

@yongtang yongtang commented Mar 5, 2018

This fix addresses expected vs actual in integration tests
so that they match assert.Equal(t, expected, actual)

Signed-off-by: Yong Tang yong.tang.github@outlook.com

Copy link
Copy Markdown
Member

@dnephin dnephin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Although I think testify is wrong here. It should be (actual, expected) to be consistent with other asserts.

This is fixed in gotestyourself/assert by allowing either, and using the variable names to report which value is expected.

This fix addresses `expected` vs `actual` in integration tests
so that they match `assert.Equal(t, expected, actual)`

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@yongtang yongtang force-pushed the 03052018-expected-actual branch from 623c69b to 8a854e9 Compare March 5, 2018 20:39
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 5, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@4b2fb7e). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #36490   +/-   ##
=========================================
  Coverage          ?   34.66%           
=========================================
  Files             ?      613           
  Lines             ?    45387           
  Branches          ?        0           
=========================================
  Hits              ?    15733           
  Misses            ?    27595           
  Partials          ?     2059

@yongtang
Copy link
Copy Markdown
Member Author

yongtang commented Mar 5, 2018

Rebased to resolve merge conflicts.

@thaJeztah
Copy link
Copy Markdown
Member

experimental failed on https://jenkins.dockerproject.org/job/Docker-PRs-experimental/39638/console; restarting

21:27:49 ----------------------------------------------------------------------
21:27:49 FAIL: docker_cli_push_test.go:206: DockerSchema1RegistrySuite.TestConcurrentPush
21:27:49 
21:27:49 docker_cli_push_test.go:207:
21:27:49     testConcurrentPush(c)
21:27:49 docker_cli_push_test.go:186:
21:27:49     c.Assert(err, checker.IsNil, check.Commentf("concurrent push failed with error: %v", err))
21:27:49 ... value *exec.ExitError = &exec.ExitError{ProcessState:(*os.ProcessState)(0xc4205d8000), Stderr:[]uint8(nil)} ("exit status 1")
21:27:49 ... concurrent push failed with error: exit status 1
21:27:49 
21:27:50 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants