Skip to content

Conversation

@alpe
Copy link
Contributor

@alpe alpe commented Nov 17, 2025

Overview

make mock-gen only to cleanup some left overs.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @alpe, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request performs a targeted cleanup of generated mock files. Specifically, it removes the mock implementations for the GasMultiplier and GasPrice methods, which are no longer required. This action streamlines the codebase by eliminating unused mock code, ensuring that the project's mocks are up-to-date and free of unnecessary cruft.

Highlights

  • Mock Cleanup: Removed outdated mock implementations for the GasMultiplier and GasPrice methods from the MockDA interface.
  • Code Maintenance: Ensured that the generated mock files accurately reflect the current interfaces, reducing boilerplate and improving code hygiene.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@alpe alpe changed the title chore: Mock gen only chore: mock gen only Nov 17, 2025
@claude
Copy link
Contributor

claude bot commented Nov 17, 2025

Claude finished @alpe's task —— View job


Code Review Complete

Tasks

  • Examine PR diff to understand what was removed
  • Review DA interface definition to verify removed methods
  • Search codebase for references to removed methods
  • Review mockery configuration
  • Provide comprehensive code review feedback

Summary

This PR correctly removes outdated mock implementations for GasMultiplier() and GasPrice() methods that were removed from the DA interface in commit 8436bf4 (PR #2822). The cleanup is appropriate and necessary to keep generated mocks in sync with the actual interface definition.

Review Findings

1. Code Quality & Architecture ✅

Positive:

  • The mock generation is correctly automated via .mockery.yaml configuration (lines 5-13)
  • Both mock files (da/internal/mocks/da.go and test/mocks/da.go) were consistently updated
  • The removal is clean with no partial deletions or orphaned code
  • Mock generation follows the testify/mockery pattern correctly
  • The generated code maintains proper structure with type-safe mock implementations

Verified:

  • The DA interface at core/da/da.go:11-38 no longer contains GasPrice() or GasMultiplier() methods
  • These methods were intentionally removed in PR refactor!: use Celestia node fees estimation system #2822 to use Celestia node's fee estimation system
  • Running make mock-gen would regenerate these exact files without the removed methods

2. Error Handling & Resilience ✅

Not applicable - This is generated mock code removal only. The actual implementations that handle gas price estimation now reside in the Celestia node integration.

3. Security ✅

No security concerns identified:

  • This is purely a cleanup of generated test mocks
  • No production code is affected
  • No sensitive data handling changes

4. Performance & Resource Efficiency ✅

Positive:

  • Removes 240 lines of unused generated code (120 lines per file)
  • Reduces maintenance burden by keeping mocks synchronized with interfaces
  • Smaller mock files mean faster compilation and reduced binary size for test builds

5. Testing & Reliability ✅

Verified:

  • The removed mock methods correspond to methods that no longer exist in the interface
  • No test code should be calling these mock methods anymore (they were removed as part of PR refactor!: use Celestia node fees estimation system #2822)
  • The remaining mock methods (Commit, Get, GetIDs, GetProofs, Submit, SubmitWithOptions, Validate) all correctly match the current DA interface

Context from git history:

  • Commit 8436bf4 removed GasPrice() and GasMultiplier() from the DA interface as part of a refactor to use Celestia node's fee estimation system
  • This PR completes that refactor by cleaning up the generated mocks

6. Documentation & Developer Experience ✅

Positive:

  • The PR title "chore: mock gen only" accurately describes the change
  • The description "make mock-gen only to cleanup some left overs" clearly indicates this is maintenance work
  • The .mockery.yaml configuration is well-documented and makes it clear how to regenerate mocks

Observation:

  • The commit message "Mock gen only" is clear but could have been slightly more descriptive (e.g., "chore: regenerate DA mocks after interface cleanup")

Recommendations

No blocking issues identified. This is a straightforward, correct cleanup PR.

Optional improvements for future consideration:

  1. Consider adding a CI check that verifies generated mocks are up-to-date with their interfaces (fails if someone modifies an interface without regenerating mocks)
  2. The mockery template configuration looks good - no changes needed there

Conclusion

LGTM ✅ - This PR correctly synchronizes the generated mock files with the current DA interface definition. The removal of GasMultiplier and GasPrice mock implementations is appropriate given these methods were removed from the interface in PR #2822. The changes are clean, consistent across both mock files, and maintain code quality standards.

The PR completes the cleanup started in the gas price refactoring work and ensures the test infrastructure remains aligned with the production interfaces.


@github-actions
Copy link
Contributor

The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed⏩ skipped✅ passed✅ passedNov 17, 2025, 4:43 PM

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request cleans up generated mock code. The removal of the GasMultiplier and GasPrice methods and their associated mock helpers from da/internal/mocks/da.go and test/mocks/da.go is correct, as these methods are no longer part of the DA interface. I have one suggestion regarding the duplicated mock files to improve the project's maintainability.

@codecov
Copy link

codecov bot commented Nov 17, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.69%. Comparing base (b940439) to head (a05da40).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2853      +/-   ##
==========================================
- Coverage   64.76%   64.69%   -0.07%     
==========================================
  Files          81       81              
  Lines        7169     7169              
==========================================
- Hits         4643     4638       -5     
- Misses       1986     1991       +5     
  Partials      540      540              
Flag Coverage Δ
combined 64.69% <ø> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tac0turtle tac0turtle added this pull request to the merge queue Nov 17, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Nov 17, 2025
@alpe alpe added this pull request to the merge queue Nov 18, 2025
@tac0turtle tac0turtle removed this pull request from the merge queue due to a manual request Nov 18, 2025
@tac0turtle tac0turtle merged commit a75b4bc into main Nov 18, 2025
33 checks passed
@tac0turtle tac0turtle deleted the alex/cleanup_mocks branch November 18, 2025 09:33
@github-project-automation github-project-automation bot moved this to Done in Evolve Nov 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants