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

feat: Adds test mock support with Mockery #287

Merged
merged 9 commits into from Mar 25, 2024
Merged

feat: Adds test mock support with Mockery #287

merged 9 commits into from Mar 25, 2024

Conversation

lantoli
Copy link
Member

@lantoli lantoli commented Mar 23, 2024

Description

We are adding Testify mocks generated by Mockery to make it easier to test use of SDK from your packages.

Example of use in Terraform Atlas Provider repo: mongodb/terraform-provider-mongodbatlas#2069

Review

Files in mockadmin folder are auto-generated.

Type of change:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Required Checklist:

  • I have signed the MongoDB CLA
  • I have added tests that prove my fix is effective or that my feature works
  • I have added any necessary documentation (if appropriate)
  • I have run make fmt and formatted my code

Further comments

@lantoli lantoli marked this pull request as ready for review March 25, 2024 08:18
@lantoli lantoli requested a review from a team as a code owner March 25, 2024 08:18
.mockery.yaml Show resolved Hide resolved
examples/README.md Outdated Show resolved Hide resolved
@@ -41,3 +41,9 @@ new-release:
update-version:
## When manually updating major version
./releaser/scripts/update-version.sh

.PHONY: generate_mocks
generate_mocks:
Copy link
Member

Choose a reason for hiding this comment

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

We need to add mocking to contributing guide

Copy link
Member

Choose a reason for hiding this comment

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

Let's make follow up PR. This is large PR anyway

Copy link
Member Author

Choose a reason for hiding this comment

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

ok

go.mod Show resolved Hide resolved

.PHONY: generate_mocks
generate_mocks:
go install github.com/vektra/mockery/v2@v2.42.1
Copy link
Member

Choose a reason for hiding this comment

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

Antipattern. If we need mockery installed let's keep it at dependency level so we can get it automatically updated etc.

Copy link
Member

Choose a reason for hiding this comment

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

Let's make follow up PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok


func TestListClusters(t *testing.T) {
// Create mock API.
clusterAPI := mockadmin.NewClustersApi(t)
Copy link
Member

Choose a reason for hiding this comment

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

I think this is very good example on how to use mockery but does not fit to example of SDK mocking.

Mocking example should use "go.mongodb.org/atlas-sdk/v20231115008/admin" interface that is replaced by mocked implementation.

Copy link
Member

Choose a reason for hiding this comment

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

Let's make follow up PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

not sure I understand, we have import "go.mongodb.org/atlas-sdk/v20231115008/admin" and the admin interface is being used in the tests:
func MyFunctionCallingListClusters(clusterAPI admin.ClustersApi) (int, error)

Copy link
Member

Choose a reason for hiding this comment

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

Apologies. I got confused by the name: MyFunctionCallingListClusters. LGTM

Copy link
Member

@wtrocki wtrocki left a comment

Choose a reason for hiding this comment

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

LGTM

All comments can be follow ups.

@wtrocki
Copy link
Member

wtrocki commented Mar 25, 2024

Do we know how much build time will increase with mocking step?

@lantoli
Copy link
Member Author

lantoli commented Mar 25, 2024

@wtrocki in my local mock generation takes a couple of seconds. Is it ok if I try Generate Preview SDK action in my branch to see this? it will include the mocks in PR #260

@wtrocki
Copy link
Member

wtrocki commented Mar 25, 2024

takes a couple of seconds.

SGTM! Thank you

Is it ok if I try Generate Preview SDK action in my branch to see this?

Let's not do it. If we see it taking longer after merge and take action if needed.

For the future reference - it is best to push generated changes + tests in separate PR. We used that pattern for SDK automation so every automation PR can be easily reviewed and referenced. No need to change this PR

lantoli and others added 3 commits March 25, 2024 12:45
Co-authored-by: Wojciech Trocki <w.trocki@mongodb.com>
@lantoli lantoli merged commit bcf87d0 into main Mar 25, 2024
4 checks passed
@lantoli lantoli deleted the mockery branch March 25, 2024 12:15
@lantoli lantoli mentioned this pull request Mar 25, 2024
8 tasks
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.

None yet

2 participants