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

Migrate to new Mockery config #5684

Merged
merged 1 commit into from
Aug 21, 2023

Conversation

harshil1973
Copy link
Contributor

@harshil1973 harshil1973 commented Aug 16, 2023

Closes #5564.

@harshil1973 harshil1973 requested a review from a team as a code owner August 16, 2023 01:49
@harshil1973 harshil1973 requested review from tsaarni and skriss and removed request for a team August 16, 2023 01:49
@github-actions
Copy link

Hi @harshil1973! Welcome to our community and thank you for opening your first Pull Request. Someone will review it soon. Thank you for committing to making Contour better. You can also join us on our mailing list and in our channel in the Kubernetes Slack Workspace

@skriss skriss added the release-note/none-required Marks a PR as not requiring a release note. Should only be used for very small changes. label Aug 16, 2023
@codecov
Copy link

codecov bot commented Aug 16, 2023

Codecov Report

Merging #5684 (c39d302) into main (a991c9c) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #5684   +/-   ##
=======================================
  Coverage   78.59%   78.59%           
=======================================
  Files         138      138           
  Lines       19150    19150           
=======================================
  Hits        15051    15051           
  Misses       3812     3812           
  Partials      287      287           

Copy link
Member

@skriss skriss left a comment

Choose a reason for hiding this comment

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

Thanks @harshil1973, this looks like a great start. We'll also need to change how we invoke the mock generation -- currently, we're calling go generate (

contour/Makefile

Lines 271 to 274 in acc15db

.PHONY: generate-go
generate-go:
@echo "Generating mocks..."
@go generate ./...
), but now we'll need to use the mockery binary instead (ref. https://vektra.github.io/mockery/v2.31/running/). Typically for these cases we'll do e.g. go run github.com/vektra/mockery/v2.

A good way to test this is to delete the existing mock Go files, then run make generate, and ensure they're recreated with no changes.

Copy link
Member

@sunjayBhatia sunjayBhatia left a comment

Choose a reason for hiding this comment

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

I think we need to add an invocation of mockery in our generate-go make target, go generate doesn't actually run it anymore

@sunjayBhatia
Copy link
Member

Thanks @harshil1973, this looks like a great start. We'll also need to change how we invoke the mock generation -- currently, we're calling go generate (

contour/Makefile

Lines 271 to 274 in acc15db

.PHONY: generate-go
generate-go:
@echo "Generating mocks..."
@go generate ./...

), but now we'll need to use the mockery binary instead (ref. https://vektra.github.io/mockery/v2.31/running/). Typically for these cases we'll do e.g. go run github.com/vektra/mockery/v2.
A good way to test this is to delete the existing mock Go files, then run make generate, and ensure they're recreated with no changes.

jinx!

Makefile Outdated Show resolved Hide resolved
Signed-off-by: Harshil Patel <harshilpatel1973@gmail.com>
Copy link
Member

@skriss skriss left a comment

Choose a reason for hiding this comment

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

nice, LGTM!

@skriss skriss requested a review from sunjayBhatia August 21, 2023 19:59
@skriss skriss merged commit 55d98f7 into projectcontour:main Aug 21, 2023
@harshil1973 harshil1973 deleted the mockery_migrate branch August 22, 2023 02:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/none-required Marks a PR as not requiring a release note. Should only be used for very small changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

migrate to new mockery config
3 participants