Skip to content

Conversation

@WangWang0226
Copy link

@WangWang0226 WangWang0226 commented Jan 16, 2026

Tracking issue

Closes #6855

Why are the changes needed?

The repository has been migrating to a standardized mocking approach using mockery with EXPECT() semantics (see PR #6829).
However, queue_service_test.go was still relying on a manually defined mock implementation and the legacy On() API from testify/mock.

Aligning queue_service_test.go with the existing mockery configuration ensures consistency, improves readability, and follows the recommended mocking best practices adopted in the codebase.

What changes were proposed in this pull request?

This PR refactors the queue service tests to fully adopt mockery-generated mocks and EXPECT()-based expectations.

Specifically:

  • Added github.com/flyteorg/flyte/v2/queue/service to .mockery.yaml so that mocks for QueueClientInterface are generated automatically.
  • Removed the manually implemented MockQueueClient from queue_service_test.go.
  • Replaced manual mock construction with mockery-generated mocks (mocks.NewQueueClientInterface(t)).
  • Migrated all mock expectations from On(...) to EXPECT().Method(...), aligning with the preferred mocking style used across the repository.
  • Kept the existing test coverage and behavior unchanged, focusing purely on refactoring and consistency.

No production code behavior was modified as part of this change.

How was this patch tested?

  • Existing unit tests in queue_service_test.go were updated to use mockery mocks and continue to pass.
  • queue_service_test.go were run locally using:
    go test ./queue/service
    
  • No additional test cases were required, as this PR is a refactor of test infrastructure only and does not change runtime behavior.

Labels

  • changed

Setup process

No special setup is required beyond the existing development environment.

Screenshots

N/A

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

Related PRs

Docs link

N/A

Signed-off-by: WangWang0226 <eeha8834@gmail.com>
@WangWang0226
Copy link
Author

Hi @machichima, could you please take a look when you have a chance? Thanks!

config:
include-auto-generated: true
all: true
dir: 'queue/service/mocks'
Copy link
Contributor

Choose a reason for hiding this comment

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

I Think we can add the settings to remove the "Interface" string in mock name. WDYT @machichima ?

mockname: "Mock{{ .InterfaceName | trimSuffix \"Interface\" }}"

Copy link
Member

Choose a reason for hiding this comment

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

I would prefer not using trimSuffix here, which can generate the mock with different name as the original one. For example, if the interface named UserInterface, using this will generate MockUser, which may cause confusion, and if we try to search UserInterface, we cannot find this mock interface

Copy link
Member

Choose a reason for hiding this comment

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

I agree that having Interface suffix is not the best Golang interface naming convention, to remove this I think we should directly modify the interface name rather than trim here. WDYT?

Copy link
Member

@machichima machichima left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you.

Go test passed locally for me:

❯ go test ./...
?       github.com/flyteorg/flyte/v2/queue/cmd  [no test files]
?       github.com/flyteorg/flyte/v2/queue/config       [no test files]
?       github.com/flyteorg/flyte/v2/queue/k8s  [no test files]
ok      github.com/flyteorg/flyte/v2/queue/service      0.018s
?       github.com/flyteorg/flyte/v2/queue/service/mocks        [no test files]
?       github.com/flyteorg/flyte/v2/queue/testclient   [no test files]

@machichima machichima merged commit ef0c2ed into flyteorg:v2 Jan 16, 2026
4 checks passed
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.

3 participants