Skip to content

fix: fixed hot-reload error propagation and added reload manager regression tests#1245

Open
Ishita-190 wants to merge 1 commit intomofa-org:mainfrom
Ishita-190:test
Open

fix: fixed hot-reload error propagation and added reload manager regression tests#1245
Ishita-190 wants to merge 1 commit intomofa-org:mainfrom
Ishita-190:test

Conversation

@Ishita-190
Copy link
Contributor

📋 Summary

This PR improves error handling in the plugin hot-reload path and adds regression tests for the reload manager.

Previously, reload operations could appear successful even if an internal step failed. This change ensures reload failures are properly returned to the caller and covered by tests.

🔗 Related Issues

Closes #1236


🧠 Context

The plugin hot-reload manager previously did not propagate errors from internal reload steps. If plugin loading, initialization, or startup failed, the reload path could still return a successful result to the caller.

This made reload failures difficult to detect and left important failure paths untested.


🛠️ Changes

Updated the reload flow in manager.rs so errors from load/init/start steps are returned instead of being silently ignored.

Ensured manual reload calls propagate failures back to the API caller.

Added explicit logging for reload failures triggered by the event loop.

Added regression tests in hot_reload_manager_tests.rs covering:

  • unloading a missing plugin (PluginNotFound)
  • reloading a missing plugin (PluginNotFound)
  • reload attempts with a missing plugin library (LoadError and ReloadFailed event)
  • baseline manager configuration and initialization behavior.

🧪 How you Tested

cargo test -p mofa-plugins --test hot_reload_manager_tests
cargo test -p mofa-plugins --tests

📸 Screenshots / Logs (if applicable)

image

⚠️ Breaking Changes

  • No breaking changes
  • Breaking change (describe below)

🧹 Checklist

Code Quality

  • Code follows Rust idioms and project conventions
  • cargo fmt run
  • cargo clippy passes without warnings

Testing

  • Tests added/updated
  • cargo test passes locally without any error

Documentation

  • Public APIs documented
  • README / docs updated (if needed)

PR Hygiene

  • PR is small and focused (one logical change)
  • Branch is up to date with main
  • No unrelated commits
  • Commit messages explain why, not only what

@Ishita-190
Copy link
Contributor Author

@LuigiGonnella please take a look at this!

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.

[ENHANCEMENT] : regression coverage gap in persistence, prompt SQL stores, and hot-reload manager critical paths

1 participant