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.

Sign up for GitHub

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

Jump to bottom

chore: Follow-up to Mockery followup #288

Merged
merged 5 commits into from
Mar 25, 2024
Merged

chore: Follow-up to Mockery followup #288

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9061144
use binary for mockery
lantoli Mar 25, 2024
1216205
TEMPORARY: test GH action
lantoli Mar 25, 2024
58e3b70
Revert "TEMPORARY: test GH action"
lantoli Mar 25, 2024
aa54626
contributing
lantoli Mar 25, 2024
5315668
using docker to allow local and gh action execution
lantoli Mar 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/autoupdate-preview.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ jobs:
- name: Run mock generation
working-directory: ./tools
if: steps.verify-changed-files.outputs.files_changed == 'true'
run: make generate_mocks
run: make generate_mocks_linux
- uses: peter-evans/create-pull-request@v6
if: steps.verify-changed-files.outputs.files_changed == 'true'
env:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/autoupdate-prod.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ jobs:
- name: Run mock generation
working-directory: ./tools
if: steps.verify-changed-files.outputs.files_changed == 'true'
run: make generate_mocks
run: make generate_mocks_linux
- uses: peter-evans/create-pull-request@v6
if: steps.verify-changed-files.outputs.files_changed == 'true'
env:
Expand Down
1 change: 1 addition & 0 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ Repository contains following SDKs
- `tools`: tooling used to generate Atlas Admin SDK. See [./tools](./tools) for more information.
- `examples`: SDK examples
- `internal`: folder for hosting non user facing SDK methods and core helpers
- `mockadmin`: [Testify mocks](https://pkg.go.dev/github.com/stretchr/testify/mock) generated with [Mockery](https://github.com/vektra/mockery) for API interfaces
Copy link
Member

@wtrocki wtrocki Mar 25, 2024 •
edited

Choose a reason for hiding this comment

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

IMHO we might need documentation that clarifies how we maintain mockery as part of the SDK.
Defining make target to regenerate locally and pointing to the tests example


## VSCode debugging configuration
VSCode developers might use following debugging configuration for transformer and go debugging
Expand Down
10 changes: 6 additions & 4 deletions tools/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,10 @@ update-version:
## When manually updating major version
./releaser/scripts/update-version.sh

.PHONY: generate_mocks
generate_mocks:
go install github.com/vektra/mockery/v2@v2.42.1
.PHONY: generate_mocks_linux
generate_mocks_linux:
rm -Rf ../mockadmin
mockery
curl -L -o mockery.tar.gz https://github.com/vektra/mockery/releases/download/v2.42.1/mockery_2.42.1_Linux_x86_64.tar.gz
Copy link
Collaborator

Choose a reason for hiding this comment

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

From this comment in the previous PR, this dependency would not be automatically updated, right?

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 your question

Copy link
Collaborator

@oarbusi oarbusi Mar 25, 2024 •
edited

Choose a reason for hiding this comment

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

Sorry, what I meant is that as far as I understand, @wtrocki wanted the mockery dependency version to be elsewhere so that it can be automatically updated by dependabot. As it is now, I don't think that's the case

Copy link
Member

Choose a reason for hiding this comment

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

Related to the #288 (comment)

Re version - I think we use pattern to define them in the header of the markdown to understand how to update them and cover them in the 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.

See: https://github.com/mongodb/atlas-sdk-go/blob/main/Makefile#L2

Copy link
Member

Choose a reason for hiding this comment

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

Alternatively we can use tools target as way to install all dependencies:
https://github.com/mongodb/atlas-sdk-go/blob/main/Makefile#L40

Copy link
Member

@wtrocki wtrocki Mar 25, 2024 •
edited

Choose a reason for hiding this comment

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

I think we can reuse some of the scripts linked below as this solution to introduce patterns for make:

  1. Cache installation (mocking locally fails
  2. Clean up files or ignore them with git)
  3. Check if tool exist (making redundant installs without cache)

We tend to use .bin file in the repo so developers do not get tar.gz file as change when running this command.

Official docs:
https://vektra.github.io/mockery/latest/installation/

Some examples of makefile efficient installations:

  1. https://github.com/bxcodec/go-clean-arch/blob/e06c6d0cb37069b0ef56e3df67f80ca130a1ab82/misc/make/tools.Makefile#L85
  2. https://github.com/hyperledger/fabric/blob/111cff51600d26d4b4b05f52825da11e7629e971/gotools.mk#L36
  3. https://github.com/tendermint/tendermint/blob/35581cf54ec436b8c37fabb43fdaa3f48339a170/scripts/mockery_generate.sh#L9-L15

Copy link
Member Author

Choose a reason for hiding this comment

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

thank you, at the end I've followed the Docker approach so it works both in local and in the GH action

tar -xzf mockery.tar.gz
chmod +x mockery
./mockery