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

test: update mockery configuration and regenerate mocks #955

Merged
merged 22 commits into from
Nov 6, 2024

Conversation

lklimek
Copy link
Collaborator

@lklimek lklimek commented Oct 11, 2024

Issue being fixed or feature implemented

Current mocking solution with mockery using go:generate is deprecated.

What was done?

  • created .mockery.yaml
  • updated Makefile and scripts/mockery_generate.sh
  • deleted and regenerated all mocks

How Has This Been Tested?

GHA

Breaking Changes

None

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

Summary by CodeRabbit

  • New Features

    • Introduced a new configuration file for mock generation, enhancing automated testing capabilities.
    • Simplified the mock generation process by modifying the existing script.
  • Bug Fixes

    • Updated dependency version for the Mockery tool, ensuring compatibility and improvements.
  • Chores

    • Removed outdated mock implementations that are no longer needed, cleaning up the codebase.

Base automatically changed from build/go-1.23 to v1.3-dev October 16, 2024 11:24
Copy link
Contributor

coderabbitai bot commented Oct 16, 2024

Walkthrough

The pull request introduces several changes primarily focused on updating mock generation configurations and simplifying testing workflows. The GitHub Actions workflow for checking generated mocks has been adjusted to streamline the error-checking process. Additionally, a new configuration file for mock generation has been added, specifying parameters for various interfaces. Several mock implementation files have been deleted, indicating a shift in testing strategy, while a script for generating mocks has been simplified to use default settings.

Changes

File Path Change Summary
.github/workflows/check-generated.yml Updated check-mocks job: changed go-version from 1.23 to 1.23.2, simplified command to git diff --exit-code.
.mockery.yaml New configuration file for mock generation with parameters for multiple interfaces and packages.
Makefile Modified mockery target to directly invoke ./scripts/mockery_generate.sh instead of using go generate.
cmd/tenderdash/commands/reindex_event_test.go Updated import for mockEventSink from mocks.EventSink to indexermocks.EventSink in TestReIndexEvent.
internal/blocksync/mocks/block_client.go Deleted mock implementation of BlockClient.
internal/consensus/mocks/fast_sync_reactor.go Deleted mock implementation of BlockSyncReactor.
internal/proxy/mocks/app_conn_mempool.go Deleted mock implementation of AppConnMempool.
internal/proxy/mocks/app_conn_query.go Deleted mock implementation of AppConnQuery.
internal/proxy/mocks/app_conn_snapshot.go Deleted mock implementation of AppConnSnapshot.
internal/state/mocks/event_sink.go Deleted mock implementation of EventSink.
internal/statesync/mocks/Metricer.go Deleted mock implementation of Metricer.
rpc/client/mocks/abci_client.go Deleted mock implementation of ABCIClient.
rpc/client/mocks/events_client.go Deleted mock implementation of EventsClient.
rpc/client/mocks/evidence_client.go Deleted mock implementation of EvidenceClient.
rpc/client/mocks/history_client.go Deleted mock implementation of HistoryClient.
rpc/client/mocks/mempool_client.go Deleted mock implementation of MempoolClient.
rpc/client/mocks/network_client.go Deleted mock implementation of NetworkClient.
rpc/client/mocks/sign_client.go Deleted mock implementation of SignClient.
rpc/client/mocks/status_client.go Deleted mock implementation of StatusClient.
rpc/client/mocks/subscription_client.go Deleted mock implementation of SubscriptionClient.
scripts/mockery_generate.sh Simplified script to directly call mockery without additional flags or parameters.
go.mod Updated github.com/vektra/mockery/v2 dependency version from v2.46.2 to v2.46.3.

Poem

🐰 In the meadow where the mocks once lay,
A new breeze whispers, "Let’s streamline the way!"
With simpler scripts and a YAML so bright,
Our tests hop along, all ready for flight.
Gone are the mocks that cluttered our view,
We celebrate changes, oh yes, it’s true! 🌼


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 10817a3 and af889e0.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (4)
  • .github/workflows/check-generated.yml (1 hunks)
  • Makefile (1 hunks)
  • cmd/tenderdash/commands/reindex_event_test.go (2 hunks)
  • go.mod (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • .github/workflows/check-generated.yml
  • Makefile
  • cmd/tenderdash/commands/reindex_event_test.go
  • go.mod

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (3)
.mockery.yaml (2)

1-9: Consider enabling with-expecter for more powerful mocking capabilities.

The global configuration settings are well-structured and follow good practices. However, you might want to consider setting with-expecter: true. This would generate expecter structs, allowing for more powerful and flexible mocking scenarios, especially useful for complex testing situations.

The rest of the configuration looks good:

  • The directory structure and naming conventions are clear and intuitive.
  • Using warn-level logging and disabling version strings will keep the output clean.
  • Setting all: false gives precise control over which interfaces are mocked.

10-71: Comprehensive package list, but consider addressing potential naming conflicts.

The package list for mock generation is thorough and covers key components of the Tenderdash project. It's great to see that various crucial interfaces are included, which will facilitate effective testing and dependency injection throughout the codebase.

However, there are two suggestions for improvement:

  1. Potential naming conflicts: Some packages (e.g., abci/client and dash/core) both have a Client interface. This might lead to naming conflicts in the generated mocks. Consider using more specific names or prefixing the mock names with the package name to avoid confusion.

  2. Add explanatory comments: It would be beneficial to add brief comments explaining the purpose of mocking each interface, especially for less obvious ones. This can help other developers understand the testing strategy and the role of each mock.

cmd/tenderdash/commands/reindex_event_test.go (1)

Line range hint 1-203: Overall assessment: Changes align with PR objectives.

The modifications in this file successfully update the mocking implementation as intended. The test logic remains unchanged, which suggests that the new mock is compatible with the existing test structure. This approach minimizes the risk of introducing new bugs while achieving the goal of updating the mocking solution.

To further improve the PR:

  1. Consider adding a comment explaining the reason for the mock update, which would help future developers understand the change.
  2. Ensure that all other files using EventSink mocks have been similarly updated for consistency.
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between f8ebd34 and 24c796a.

📒 Files selected for processing (23)
  • .github/workflows/check-generated.yml (2 hunks)
  • .mockery.yaml (1 hunks)
  • Makefile (1 hunks)
  • cmd/tenderdash/commands/reindex_event_test.go (2 hunks)
  • dash/core/mocks/client.go (3 hunks)
  • internal/blocksync/mocks/block_client.go (0 hunks)
  • internal/consensus/mocks/fast_sync_reactor.go (0 hunks)
  • internal/proxy/mocks/app_conn_mempool.go (0 hunks)
  • internal/proxy/mocks/app_conn_query.go (0 hunks)
  • internal/proxy/mocks/app_conn_snapshot.go (0 hunks)
  • internal/state/mocks/event_sink.go (0 hunks)
  • internal/statesync/mocks/Metricer.go (0 hunks)
  • rpc/client/mocks/abci_client.go (0 hunks)
  • rpc/client/mocks/events_client.go (0 hunks)
  • rpc/client/mocks/evidence_client.go (0 hunks)
  • rpc/client/mocks/history_client.go (0 hunks)
  • rpc/client/mocks/mempool_client.go (0 hunks)
  • rpc/client/mocks/network_client.go (0 hunks)
  • rpc/client/mocks/sign_client.go (0 hunks)
  • rpc/client/mocks/status_client.go (0 hunks)
  • rpc/client/mocks/subscription_client.go (0 hunks)
  • scripts/mockery_generate.sh (1 hunks)
  • types/mocks/privvalidator.go (15 hunks)
💤 Files with no reviewable changes (16)
  • internal/blocksync/mocks/block_client.go
  • internal/consensus/mocks/fast_sync_reactor.go
  • internal/proxy/mocks/app_conn_mempool.go
  • internal/proxy/mocks/app_conn_query.go
  • internal/proxy/mocks/app_conn_snapshot.go
  • internal/state/mocks/event_sink.go
  • internal/statesync/mocks/Metricer.go
  • rpc/client/mocks/abci_client.go
  • rpc/client/mocks/events_client.go
  • rpc/client/mocks/evidence_client.go
  • rpc/client/mocks/history_client.go
  • rpc/client/mocks/mempool_client.go
  • rpc/client/mocks/network_client.go
  • rpc/client/mocks/sign_client.go
  • rpc/client/mocks/status_client.go
  • rpc/client/mocks/subscription_client.go
✅ Files skipped from review due to trivial changes (1)
  • scripts/mockery_generate.sh
🧰 Additional context used
🔇 Additional comments (19)
.github/workflows/check-generated.yml (2)

38-38: Simplified diff check for generated mocks

The removal of the --stat option from the git diff command simplifies the check. This change focuses the output solely on whether there are differences, without providing statistics about the changes.

This simplification is beneficial as it:

  1. Reduces noise in the CI output
  2. Maintains the core functionality of detecting changes in generated mocks
  3. Potentially speeds up the check process slightly

The error message (lines 39-44) still provides clear instructions on what to do if changes are detected, so no important information is lost with this simplification.


26-26: Verify the Go version number

The update to a newer Go version is good practice. However, the version "1.23.2" seems unusual, as Go versions typically follow a different pattern (e.g., 1.20, 1.21, etc.).

Please confirm that this is the correct version number. If it's a typo, consider updating it to the latest stable Go version.

To verify the latest stable Go version, you can run:

✅ Verification successful

Go version "1.23.2" verified as the latest stable release.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Fetch the latest stable Go version
curl -sSL https://golang.org/VERSION?m=text

Length of output: 82

cmd/tenderdash/commands/reindex_event_test.go (2)

16-16: LGTM: New import for updated mocking solution.

The addition of the imocks import aligns with the PR objective of updating the mocking solution. This change is consistent with the migration from the current deprecated mocking solution to the new configuration.


140-140: LGTM: Updated mock type consistent with new import.

The change from *mocks.EventSink to *imocks.EventSink is consistent with the new import and aligns with the PR objectives of updating the mocking solution.

To ensure consistency across the codebase, please run the following script to verify that all EventSink mocks have been updated:

✅ Verification successful

LGTM: Updated mock type in reindex_event_test.go is correct.
The change from *mocks.EventSink to *imocks.EventSink in cmd/tenderdash/commands/reindex_event_test.go aligns with the PR objectives.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all EventSink mocks have been updated to use imocks.

# Test: Search for any remaining uses of mocks.EventSink
rg --type go 'mocks\.EventSink'

# Test: Confirm new usage of imocks.EventSink
rg --type go 'imocks\.EventSink'

Length of output: 1006

dash/core/mocks/client.go (3)

9-10: LGTM: New import added correctly.

The new import for the crypto package is correctly added and is necessary for the updated QuorumInfo method signature.


Line range hint 155-170: LGTM: Method body updated consistently.

The QuorumInfo method body has been correctly updated to use the new crypto.QuorumHash type in type assertions and function calls. These changes are consistent with the method signature update and maintain the existing error handling and return logic.


Line range hint 1-240: Summary: Mock file updated successfully.

The changes to this mock file are consistent with the PR objectives. The QuorumInfo method has been updated to use the new crypto.QuorumHash type, and all related code has been adjusted accordingly. These changes should improve type safety and consistency in the project.

To ensure full compatibility, please verify that:

  1. The actual Client interface in the main codebase has been updated to match this mock.
  2. Any tests using this mock have been updated to use the new crypto.QuorumHash type.

You can use the following script to check for the interface definition:

#!/bin/bash
# Search for the Client interface definition
rg --type go 'type\s+Client\s+interface' -A 20
types/mocks/privvalidator.go (11)

28-36: LGTM: Method signature and implementation updated correctly

The ExtractIntoValidator method has been successfully updated to use crypto.QuorumHash instead of bytes.HexBytes for the quorumHash parameter. The implementation logic has been adjusted accordingly to handle the new type.

Also applies to: 36-43


48-55: LGTM: Return type and implementation updated correctly

The GetFirstQuorumHash method has been successfully updated to return crypto.QuorumHash instead of bytes.HexBytes. The implementation logic has been adjusted accordingly to handle the new return type.

Also applies to: 55-57, 57-64, 64-75


Line range hint 78-87: LGTM: Method signature and implementation updated correctly

The GetHeight method has been successfully updated to use crypto.QuorumHash instead of bytes.HexBytes for the quorumHash parameter. The implementation logic has been adjusted accordingly to handle the new type.

Also applies to: 87-90, 90-96, 96-103


Line range hint 106-115: LGTM: Method signature and implementation updated correctly

The GetPrivateKey method has been successfully updated to use crypto.QuorumHash instead of bytes.HexBytes for the quorumHash parameter. The implementation logic has been adjusted accordingly to handle the new type.

Also applies to: 115-118, 118-126, 126-133


136-143: LGTM: Return type and implementation updated correctly

The GetProTxHash method has been successfully updated to return crypto.ProTxHash instead of bytes.HexBytes. The implementation logic has been adjusted accordingly to handle the new return type.

Also applies to: 143-145, 145-148, 148-152, 152-163


Line range hint 166-175: LGTM: Method signature and implementation updated correctly

The GetPubKey method has been successfully updated to use crypto.QuorumHash instead of bytes.HexBytes for the quorumHash parameter. The implementation logic has been adjusted accordingly to handle the new type.

Also applies to: 175-178, 178-186, 186-193


Line range hint 196-205: LGTM: Method signature and implementation updated correctly

The GetThresholdPublicKey method has been successfully updated to use crypto.QuorumHash instead of bytes.HexBytes for the quorumHash parameter. The implementation logic has been adjusted accordingly to handle the new type.

Also applies to: 205-208, 208-216, 216-223


Line range hint 226-235: LGTM: Method signature and implementation updated correctly

The SignProposal method has been successfully updated to use crypto.QuorumHash instead of bytes.HexBytes for the quorumHash parameter. The implementation logic has been adjusted accordingly to handle the new type.

Also applies to: 235-238, 238-246, 246-253


256-264: LGTM: Method signature and implementation updated correctly

The SignVote method has been successfully updated to use crypto.QuorumHash instead of bytes.HexBytes for the quorumHash parameter. The implementation logic has been adjusted accordingly to handle the new type.

Also applies to: 264-270


274-276: LGTM: Method signature updated correctly

The UpdatePrivateKey method has been successfully updated to use crypto.QuorumHash instead of bytes.HexBytes for the quorumHash parameter.


Line range hint 1-292: Summary: Successful update of PrivValidator mock

This review has confirmed that all method signatures in the PrivValidator mock have been successfully updated to use crypto.QuorumHash instead of bytes.HexBytes for the quorumHash parameter. The implementation logic for each method has been adjusted accordingly to handle the new type.

These changes are consistent with the PR objectives of updating the mocking solution and align with the shift from bytes.HexBytes to crypto.QuorumHash in the main codebase. The updates should ensure that the mock accurately reflects the behavior of the actual PrivValidator implementation.

No issues or inconsistencies were found in the changes. The mock should now be compatible with the updated interfaces in the main codebase.

Makefile (1)

339-339: LGTM. Verify mock generation consistency.

The simplification of the mockery target by directly calling the script is a good change. It aligns with the PR objectives of updating the mockery configuration.

To ensure this change doesn't affect the mock generation process or its output, please run the following verification:

This script will help verify that the mock generation process produces consistent results with the new implementation.

dash/core/mocks/client.go Outdated Show resolved Hide resolved
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the Stale label Oct 27, 2024
@github-actions github-actions bot closed this Nov 1, 2024
@lklimek lklimek reopened this Nov 2, 2024
@lklimek lklimek removed the Stale label Nov 2, 2024
@lklimek lklimek added the S:wip label Nov 2, 2024
@lklimek
Copy link
Collaborator Author

lklimek commented Nov 4, 2024

@coderabbitai review all

Copy link
Contributor

coderabbitai bot commented Nov 4, 2024

@lklimek: I will review all the changes in the PR.

✅ Actions performed

Full review triggered.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Outside diff range and nitpick comments (3)
scripts/mockery_generate.sh (1)

Line range hint 1-8: Update script documentation to reflect current behavior.

The script header comments should be updated to:

  1. Mention that configuration is now handled via .mockery.yaml
  2. Remove the reference to accepting type arguments
  3. Add information about the simplified workflow
 #!/bin/sh
 #
-# Invoke Mockery v2 to update generated mocks for the given type.
+# Invoke Mockery v2 to update generated mocks using configuration from .mockery.yaml
 #
 # This script runs a locally-installed "mockery" if available, otherwise it
 # runs the published Docker container. This legerdemain is so that the CI build
 # and a local build can work off the same script.
 #
.mockery.yaml (1)

23-25: Consider adding comments for specialized interfaces.

The ProposerSelector interface in the versioned consensus package might benefit from a comment explaining its role in the version-specific proposer selection mechanism.

.github/workflows/check-generated.yml (1)

Consider consistent version pinning across all tools

The analysis shows inconsistent version pinning practices:

  1. Most GitHub Actions are properly pinned to specific versions (e.g., actions/checkout@v4, actions/[email protected])
  2. Mockery is pinned to a specific version through ${MOCKERY} variable
  3. However, buf and gogo protobuf tools are installed using:
    go install github.com/bufbuild/buf/cmd/buf
    go install github.com/gogo/protobuf/protoc-gen-gogofaster@latest

To maintain consistent builds and prevent unexpected breaking changes, consider:

  • Pin buf to a specific version using @vX.Y.Z
  • Pin gogo protobuf to a specific version instead of using @latest
🔗 Analysis chain

Line range hint 31-33: Consider consistent version pinning strategy

While mockery is pinned to a specific version (2.46.2), buf and gogo protobuf tools use the latest version. Consider pinning all tool versions to ensure reproducible builds and prevent unexpected breaking changes.

Let's check if there are any version constraints in other configuration files:

Also applies to: 65-66


Let me analyze the version pinning strategy across the repository by checking other workflow files:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Look for tool version specifications in configuration files

# Check common configuration files for version specifications
echo "Checking configuration files for version specifications..."
rg -i "version|mockery|buf|gogo" -g "*.{yaml,yml,toml,json}" --no-heading

Length of output: 157384


Script:

#!/bin/bash
# Check all GitHub workflow files for version pinning patterns
echo "Checking GitHub workflow files for version pinning..."
rg -U "uses:|run:.*version|mockery|buf|gogo" .github/workflows/

Length of output: 5592

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 41036c8 and a2b7ed4.

📒 Files selected for processing (21)
  • .github/workflows/check-generated.yml (2 hunks)
  • .mockery.yaml (1 hunks)
  • Makefile (1 hunks)
  • cmd/tenderdash/commands/reindex_event_test.go (2 hunks)
  • internal/blocksync/mocks/block_client.go (0 hunks)
  • internal/consensus/mocks/fast_sync_reactor.go (0 hunks)
  • internal/proxy/mocks/app_conn_mempool.go (0 hunks)
  • internal/proxy/mocks/app_conn_query.go (0 hunks)
  • internal/proxy/mocks/app_conn_snapshot.go (0 hunks)
  • internal/state/mocks/event_sink.go (0 hunks)
  • internal/statesync/mocks/Metricer.go (0 hunks)
  • rpc/client/mocks/abci_client.go (0 hunks)
  • rpc/client/mocks/events_client.go (0 hunks)
  • rpc/client/mocks/evidence_client.go (0 hunks)
  • rpc/client/mocks/history_client.go (0 hunks)
  • rpc/client/mocks/mempool_client.go (0 hunks)
  • rpc/client/mocks/network_client.go (0 hunks)
  • rpc/client/mocks/sign_client.go (0 hunks)
  • rpc/client/mocks/status_client.go (0 hunks)
  • rpc/client/mocks/subscription_client.go (0 hunks)
  • scripts/mockery_generate.sh (1 hunks)
💤 Files with no reviewable changes (16)
  • internal/blocksync/mocks/block_client.go
  • internal/consensus/mocks/fast_sync_reactor.go
  • internal/proxy/mocks/app_conn_mempool.go
  • internal/proxy/mocks/app_conn_query.go
  • internal/proxy/mocks/app_conn_snapshot.go
  • internal/state/mocks/event_sink.go
  • internal/statesync/mocks/Metricer.go
  • rpc/client/mocks/abci_client.go
  • rpc/client/mocks/events_client.go
  • rpc/client/mocks/evidence_client.go
  • rpc/client/mocks/history_client.go
  • rpc/client/mocks/mempool_client.go
  • rpc/client/mocks/network_client.go
  • rpc/client/mocks/sign_client.go
  • rpc/client/mocks/status_client.go
  • rpc/client/mocks/subscription_client.go
🔇 Additional comments (7)
scripts/mockery_generate.sh (1)

16-16: Verify the impact of removing command-line arguments.

The script no longer accepts or passes arguments to mockery, which might affect existing workflows.

✅ Verification successful

Let me analyze the git history to understand the impact of removing command-line arguments from the mockery script.


Based on the output, I can see that mockery_generate.sh is used extensively throughout the codebase in //go:generate directives with arguments to generate mock implementations of interfaces. The change to remove command-line arguments would break all these usages.

Let me verify the current mockery configuration to understand if there's a config file that might be handling these interface names instead.


Let me check the contents of the mockery configuration file since we found one at .mockery.yaml.


Based on the findings, I can now provide a final response:

The removal of command-line arguments is safe as mockery configuration has been moved to .mockery.yaml

The recent changes show that mockery has been upgraded to use a configuration file (.mockery.yaml) that explicitly lists all interfaces that need to be mocked. This file was added in commit 1f64216 and contains all the interfaces that were previously specified via command-line arguments in the //go:generate directives. The script can now run mockery without arguments as all necessary configuration is in the YAML file.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for potential usage of command-line arguments in workflows or documentation

# Test 1: Check if the script is called with arguments in any workflow files
echo "Checking workflow files for script usage with arguments:"
rg -l "mockery_generate\.sh.*[^\"'\s]" .github/workflows/

# Test 2: Check if there are any documentation references to passing arguments
echo "Checking documentation for references to script arguments:"
rg -l "mockery_generate\.sh.*type" docs/
rg -l "mockery_generate\.sh.*interface" docs/

# Test 3: Check Makefile for any usage with arguments
echo "Checking Makefile for script usage with arguments:"
rg -l "mockery_generate\.sh.*[^\"'\s]" Makefile

Length of output: 565


Script:

#!/bin/bash
# Let's try a different approach to find any usage of the script

# Check for any references to the script in the entire codebase
echo "Checking for any references to mockery_generate.sh:"
rg -l "mockery_generate.sh" 

# Check the git history for previous versions of the script
echo -e "\nChecking git history for the script:"
git log -p scripts/mockery_generate.sh

# Check if there are any make targets that use this script
echo -e "\nChecking Makefile contents:"
cat Makefile

Length of output: 277908


Script:

#!/bin/bash
# Let's check if there are any references to mockery_generate.sh with arguments in the codebase

# Check Makefile for any usage
echo "Checking Makefile for mockery usage:"
cat Makefile

# Check for any references to mockery_generate.sh with arguments in the entire codebase
echo -e "\nChecking for any references to mockery_generate.sh with arguments:"
rg "mockery_generate\.sh.*[^\"'\s]"

Length of output: 17930


Script:

#!/bin/bash
# Check for mockery configuration files
echo "Checking for mockery config files:"
find . -name ".mockery.yaml"

# Check the mockery version being used
echo -e "\nChecking mockery version:"
mockery --version

# Look at the generated mocks to see if they're still being generated correctly
echo -e "\nChecking recently generated mock files:"
find . -name "mock_*.go" -type f -mtime -7

Length of output: 459


Script:

#!/bin/bash
# Check the contents of the mockery config file
echo "Contents of .mockery.yaml:"
cat .mockery.yaml

# Also check if there are any recent changes to this file
echo -e "\nRecent changes to .mockery.yaml:"
git log -p .mockery.yaml

Length of output: 4810

.mockery.yaml (3)

1-9: Consider enabling expecter functionality for enhanced testing capabilities.

The current configuration disables the expecter functionality (with-expecter: false), which might limit the ability to write more expressive tests. The expecter pattern provides a more fluent API for setting up expectations in tests.

Additionally:

  • Consider using a more verbose log level during development
  • Verify that the directory template {{.InterfaceDir}}/mocks works correctly with your project structure
#!/bin/bash
# Description: Verify the mocks directory structure and check for any existing mocks that might be affected

# Test 1: Check if mocks directories exist in the expected locations
echo "Checking mocks directories..."
fd -t d "mocks$" 

# Test 2: Verify no old mock files remain
echo "Checking for old mock files..."
fd -e go . | rg -i "mock_.*\.go$"

37-42: Verify P2P client interface hierarchy.

The P2P client package defines multiple specialized clients (Client, BlockClient, SnapshotClient). Ensure these interfaces maintain proper separation of concerns and don't have overlapping responsibilities.

#!/bin/bash
# Description: Check P2P client interface definitions for potential overlap

echo "Analyzing P2P client interfaces..."
ast-grep --pattern 'type $client interface {
  $$$
}'

10-71: Verify package paths and interface completeness.

The configuration includes a comprehensive list of packages and interfaces for mock generation. However, we should verify:

  1. All package paths are valid
  2. No critical interfaces are missing
  3. Interface names match the actual code
✅ Verification successful

All interfaces in .mockery.yaml are correctly mapped to their implementations

The verification confirms that all interfaces listed in .mockery.yaml exist in their respective packages:

  1. ABCI interfaces:

    • Client in abci/client
    • Application in abci/types
  2. Core interfaces:

    • Client in dash/core
  3. Consensus interfaces:

    • Gossiper in internal/consensus
    • ProposerSelector in internal/consensus/versioned/selectproposer
  4. P2P interfaces:

    • Transport, Connection, Channel in internal/p2p
    • Client interfaces in internal/p2p/client
  5. State interfaces:

    • BlockStore in internal/state
    • Store in internal/state
    • EventSink in internal/state/indexer
    • StateProvider in internal/statesync
  6. Light client interfaces:

    • Provider in light/provider
    • LightClient in light/rpc
  7. RPC interfaces:

    • Client and RemoteClient in rpc/client
  8. Mempool interface:

    • Mempool in internal/mempool/types
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify package paths and interface declarations

# Test 1: Verify package paths exist
echo "Verifying package paths..."
while IFS= read -r pkg; do
  if [[ $pkg =~ ^[[:space:]]+[a-z] ]]; then
    pkg_path=$(echo "$pkg" | sed 's/^[[:space:]]*//;s/:.*$//')
    echo "Checking: $pkg_path"
    fd -t f "\.go$" | rg "$pkg_path"
  fi
done < .mockery.yaml

# Test 2: Verify interface declarations
echo "Verifying interfaces..."
while IFS= read -r line; do
  if [[ $line =~ ^[[:space:]]+[A-Z] ]]; then
    interface_name=$(echo "$line" | tr -d ':' | xargs)
    echo "Checking interface: $interface_name"
    ast-grep --pattern "type $interface_name interface"
  fi
done < .mockery.yaml

Length of output: 307


Script:

#!/bin/bash
# Let's verify package paths and interfaces separately with more robust commands

# Check package paths
echo "=== Checking Package Paths ==="
rg -l "package (client|types|core|consensus|selectproposer|evidence|mempool|p2p|state|indexer|statesync|store|provider|rpc)" --type go

# Check interfaces
echo -e "\n=== Checking Interfaces ==="
# ABCI Client
echo "Checking ABCI Client interface..."
ast-grep --pattern 'type Client interface'

# Application
echo -e "\nChecking Application interface..."
ast-grep --pattern 'type Application interface'

# Gossiper
echo -e "\nChecking Gossiper interface..."
ast-grep --pattern 'type Gossiper interface'

# ProposerSelector
echo -e "\nChecking ProposerSelector interface..."
ast-grep --pattern 'type ProposerSelector interface'

# Mempool
echo -e "\nChecking Mempool interface..."
ast-grep --pattern 'type Mempool interface'

# P2P interfaces
echo -e "\nChecking P2P interfaces..."
ast-grep --pattern 'type Transport interface'
ast-grep --pattern 'type Connection interface'
ast-grep --pattern 'type Channel interface'

# State interfaces
echo -e "\nChecking State interfaces..."
ast-grep --pattern 'type BlockStore interface'
ast-grep --pattern 'type Store interface'
ast-grep --pattern 'type EventSink interface'
ast-grep --pattern 'type StateProvider interface'

# Light client interfaces
echo -e "\nChecking Light client interfaces..."
ast-grep --pattern 'type Provider interface'
ast-grep --pattern 'type LightClient interface'

Length of output: 12102


Script:

#!/bin/bash
# Let's search for interfaces with more context to verify their package locations

echo "=== Searching for interfaces with package context ==="

# ABCI interfaces
echo "1. ABCI Interfaces:"
rg "type (Client|Application) interface" -B 1 --type go

# Consensus interfaces
echo -e "\n2. Consensus Interfaces:"
rg "type (Gossiper|ProposerSelector) interface" -B 1 --type go

# P2P interfaces
echo -e "\n3. P2P Interfaces:"
rg "type (Transport|Connection|Channel) interface" -B 1 --type go

# State and Store interfaces
echo -e "\n4. State & Store Interfaces:"
rg "type (BlockStore|Store|EventSink|StateProvider) interface" -B 1 --type go

# Client interfaces
echo -e "\n5. Client Interfaces:"
rg "type (Provider|LightClient|RemoteClient) interface" -B 1 --type go

# Mempool interface
echo -e "\n6. Mempool Interface:"
rg "type Mempool interface" -B 1 --type go

Length of output: 3804

.github/workflows/check-generated.yml (1)

38-38: LGTM! Simplified git diff command

Removing the --stat flag is a good optimization as it simplifies the output while maintaining the same functionality. The error message still provides clear instructions for developers.

cmd/tenderdash/commands/reindex_event_test.go (1)

16-16: LGTM: Import change aligns with mockery update.

The addition of the new imocks import with an appropriate alias maintains clear separation between old and new mock implementations.

Makefile (1)

339-339: LGTM! Verify script permissions.

The simplified command aligns with the PR's objective of updating the mockery configuration. The change from using go generate to directly executing the script is a valid approach.

Let's verify the script permissions and integration:

✅ Verification successful

Script permissions and mockery configuration verified

The verification confirms:

  • The script scripts/mockery_generate.sh is executable
  • The .mockery.yaml configuration file exists
  • The script properly handles mockery version check and execution

The change from go generate to direct script execution is safe and well-implemented.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify script permissions and integration with new config

# Test 1: Check if the script is executable
if [[ ! -x "scripts/mockery_generate.sh" ]]; then
  echo "Error: scripts/mockery_generate.sh is not executable"
  exit 1
fi

# Test 2: Check if .mockery.yaml exists and is valid YAML
if [[ ! -f ".mockery.yaml" ]]; then
  echo "Error: .mockery.yaml not found"
  exit 1
fi

# Test 3: Verify script content
echo "Script content:"
cat scripts/mockery_generate.sh

Length of output: 566

scripts/mockery_generate.sh Show resolved Hide resolved
.github/workflows/check-generated.yml Outdated Show resolved Hide resolved
cmd/tenderdash/commands/reindex_event_test.go Outdated Show resolved Hide resolved
@lklimek lklimek enabled auto-merge (squash) November 6, 2024 08:04
@lklimek lklimek merged commit d28d5a8 into v1.3-dev Nov 6, 2024
20 checks passed
@lklimek lklimek deleted the fix/mockery branch November 6, 2024 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants