Skip to content

use mockery to create Docker API mocks, a part of #587#588

Merged
NicolasMahe merged 5 commits intodevfrom
fature/container-docker-mocks
Nov 29, 2018
Merged

use mockery to create Docker API mocks, a part of #587#588
NicolasMahe merged 5 commits intodevfrom
fature/container-docker-mocks

Conversation

@ilgooz
Copy link
Copy Markdown
Contributor

@ilgooz ilgooz commented Nov 27, 2018

No description provided.

@ilgooz ilgooz force-pushed the fature/container-docker-mocks branch from 28375a8 to 6569561 Compare November 27, 2018 08:37
@ilgooz ilgooz requested a review from a team November 27, 2018 08:39
Comment thread utils/docker/docker.go Outdated
@@ -0,0 +1,10 @@
package dockermock
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove this file, you can generate mock directly from vendor

mockery -name CommonAPIClient -dir vendor/github.com/docker/docker/client -inpkg

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I didn't want to access to vendor folder directly. It's a bit messy and we can easily avoid it like with the current implementation.

Comment thread scripts/build-mocks.sh Outdated
mockery -name=Container -dir ./container -output ./container/mocks

# generate mocks for docker.CommonAPIClient that used by container package.
mockery -name=Docker -dir ./utils/docker -inpkg
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

out file - mock_Docker.go -> don't use capital letters in filename.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I couldn't see a configuration related to this in mockery's cli. You can open an issue on their repo page. But I think it's OK this way.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

-case underscore

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I also thinking about adding -testonly flag to all mocks, but we can start with this one.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tried it and applied it to all other mocks but as I see, it messes with the interface names as well. For example, ServiceDB.go mock file for ServiceDB interface becomes to service_db.go and I don't like this naming. I prefer to skip setting the case, it's not that important anyway.

@ilgooz ilgooz mentioned this pull request Nov 29, 2018
@NicolasMahe
Copy link
Copy Markdown
Member

@krhubert can you review this pr please

@NicolasMahe NicolasMahe merged commit 369e403 into dev Nov 29, 2018
@NicolasMahe NicolasMahe deleted the fature/container-docker-mocks branch November 29, 2018 10:06
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.

3 participants