Skip to content

chore: refactor testplugins to be native go#274

Open
minh-nghia wants to merge 1 commit intomainfrom
chore/native-test-plugins
Open

chore: refactor testplugins to be native go#274
minh-nghia wants to merge 1 commit intomainfrom
chore/native-test-plugins

Conversation

@minh-nghia
Copy link
Copy Markdown
Contributor

@minh-nghia minh-nghia commented Apr 28, 2026 •
edited
Loading

Summary by CodeRabbit

  • Refactor
    • Restructured internal plugin registry abstraction to use service API interfaces instead of concrete registry implementations. This internal refactoring streamlines plugin dependency injection throughout the system while maintaining existing functionality and behavior.

Also removes the need to build any plugin binary before running tests.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 28, 2026 •
edited
Loading

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 05eb1ce5-b68b-4ba9-828e-6b3a1e2a798f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Walkthrough

The pull request refactors the plugin registry dependency throughout the codebase. It replaces the concrete cmkpluginregistry.Registry pointer type with an interface-based serviceapi.Registry type from the service API package. Test utilities are updated to use testutils.NewTestPlugins() directly instead of manually constructing registries through plugin configuration. Test plugin implementations are migrated from proto-based catalog plugins to concrete service interface implementations. Configuration-driven plugin wiring via config.Config.Plugins is removed in favor of direct service initialization in tests.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@minh-nghia minh-nghia force-pushed the chore/native-test-plugins branch 2 times, most recently from c317a8b to 3bbf4e3 Compare April 28, 2026 12:33
@minh-nghia minh-nghia marked this pull request as ready for review April 28, 2026 12:39
@minh-nghia
Copy link
Copy Markdown
Contributor Author

minh-nghia commented Apr 28, 2026

@coderabbitai summary

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 28, 2026

✅ Actions performed

Summary regeneration triggered.

@minh-nghia minh-nghia force-pushed the chore/native-test-plugins branch from 3bbf4e3 to ecdaaf7 Compare April 28, 2026 13:08
@minh-nghia minh-nghia force-pushed the chore/native-test-plugins branch from ecdaaf7 to ea8fcff Compare April 28, 2026 13:40
jmpTeixeira02
jmpTeixeira02 approved these changes Apr 30, 2026
View reviewed changes
Comment thread internal/manager/workflow_test.go

svcRegistry, err := cmkpluginregistry.New(t.Context(), cfg, cmkpluginregistry.WithBuiltInPlugins(ps))
assert.NoError(t, err)
cfg.Plugins = nil
Copy link
Copy Markdown
Contributor

@jmpTeixeira02 jmpTeixeira02 Apr 30, 2026

Choose a reason for hiding this comment

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

This can be deleted

Comment thread internal/manager/system_test.go
svcRegistry, err := cmkpluginregistry.New(t.Context(), &cfg, cmkpluginregistry.WithBuiltInPlugins(ps))
require.NoError(t, err)
svcRegistry := testutils.NewTestPlugins()
var err error
Copy link
Copy Markdown
Contributor

@jmpTeixeira02 jmpTeixeira02 Apr 30, 2026

Choose a reason for hiding this comment

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

Can delete this var err

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jmpTeixeira02 jmpTeixeira02 jmpTeixeira02 approved these changes

@bkieling bkieling Awaiting requested review from bkieling

@anettlippert anettlippert Awaiting requested review from anettlippert

@nnicora nnicora Awaiting requested review from nnicora

@doanhluongsap doanhluongsap Awaiting requested review from doanhluongsap

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

test tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@minh-nghia @jmpTeixeira02