Skip to content
This repository has been archived by the owner on Mar 21, 2022. It is now read-only.

Update SwarmJoin non/nullable fields #796

Merged
merged 1 commit into from
Jun 27, 2017
Merged

Conversation

fdonze
Copy link
Contributor

@fdonze fdonze commented Jun 13, 2017

SwarmJoin.AdvertiseAddr field is Nullable but RemoteAddrs and JoinToken are not

@codecov-io
Copy link

codecov-io commented Jun 16, 2017

Codecov Report

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

@@            Coverage Diff            @@
##             master     #796   +/-   ##
=========================================
  Coverage          ?   66.04%           
  Complexity        ?      713           
=========================================
  Files             ?      162           
  Lines             ?     3060           
  Branches          ?      350           
=========================================
  Hits              ?     2021           
  Misses            ?      884           
  Partials          ?      155

@davidxia
Copy link
Contributor

Is there an easy way to write a test for this?

@fdonze
Copy link
Contributor Author

fdonze commented Jun 25, 2017

@davidxia An integration test would need a second node, I do not know enough about the test infrastructure to know how feasible/easy it is. I would need guidance on this.

@davidxia
Copy link
Contributor

Yea I thought an integration test would be hard. How about a unit test then? See DefaultDockerClientUnitTest. The test failure is caused by https://github.com/spotify/docker-client/pulls

@fdonze
Copy link
Contributor Author

fdonze commented Jun 26, 2017

@davidxia I added a unit test for joinSwarm. Not sure if it makes sense to add more tests for each nullable fields as it would just test the Nullable annotation handling. Wdyt?

@davidxia davidxia merged commit 3182712 into spotify:master Jun 27, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants