Defect #38707
closed
Fix order of loading plugins' config/routes.rb
0%
Description
When loading init.rb, uses sort (source:tags/5.0.5/lib/redmine/plugin_loader.rb#L117), but loading plugin's config/routes.rb does not uses sort (source:tags/5.0.5/config/routes.rb#L400). In ruby-2.7 or earlier, Dir.glob does not sort. So, loading init.rb and loading plugin's config/routes.rb are different.
This issue includes following patches:
- 0001-fix-order-of-loading-plugins-config-routes.rb.patch : minimal changes to fix with tests.
- 0002-refactor-to-more-simply-code.patch : refactor related codes.
Files
Related issues
Updated by
Updated by Marius BĂLTEANU 6 months ago
- Status changed from Closed to Reopened
There is a random failing test caused by this.
Can we take a look, please?
Failure:
RoutingPluginsTest#test_plugins [/var/lib/jenkins/workspace/trunk/DATABASE_ADAPTER/mysql/RUBY_VER/ruby-3.1/test/test_helper.rb:323]:
A route matches "/plugin_articles", but references missing controller: PluginArticlesController
rails test test/integration/routing/plugins_test.rb:63
Updated by Marius BĂLTEANU 6 months ago
- Status changed from Reopened to Closed
The random failing test most probably was caused by the custom plugins path defined in plugin_test.rb or plugin_loader_test.rb. I think I've fixed it by explicit setting the plugins directory to tmp/test/plugins.
Updated by Vincent Robert 5 months ago
I'm investigating an issue that appears to be related to this change.
When running core tests with Redmine 5.1 and one plugin, I observe some random failures.
Here is an error when testing with the 'redmine_customize_core_fields' plugin (https://github.com/nanego/redmine_customize_core_fields) :
url_for: {:controller=>"/core_fields"}
Error:
CustomFieldEnumerationsControllerTest#test_destroy_enumeration_in_use_should_display_destroy_form:
ActionView::Template::Error: No route matches {:action=>"index", :controller=>"core_fields", :custom_field_id=>"858", :id=>"1959"}
lib/redmine/menu_manager.rb:190:in `render_single_menu_node'
More failing tests can be seen here: https://github.com/nanego/redmine_customize_core_fields/actions
The errors do not appear when running Redmine at commit 7ba9cb1da6364aba9500ec60bcb25b9ad93e6773 (branch 5.1). But it shows up when I add the two following commits and run at e5034f5ac0cd1992a8ff341e69441de407a06198
Updated by Vincent Robert 5 months ago
The core tests are passing again when I comment these two lines:
+++ b/test/integration/routing/plugins_test.rb
@@ -53,7 +53,7 @@ class RoutingPluginsTest < Redmine::RoutingTest
# Change plugin loader's directory for testing
- Redmine::PluginLoader.directory = @tmp_plugins_path
+ # Redmine::PluginLoader.directory = @tmp_plugins_path
Redmine::PluginLoader.load
Redmine::PluginLoader.directories.each(&:run_initializer) # to define relative controllers
RedmineApp::Application.instance.routes_reloader.reload!
___
def setup_plugin(plugin_name, **relative_path_to_content)
- Redmine::Plugin.directory = @tmp_plugins_path
+ # Redmine::Plugin.directory = @tmp_plugins_path
plugin_path = Redmine::Plugin.directory / plugin_name.to_s
plugin_path.mkpath
I finally understand the reason behind this issue.
Due to route reloading after changing the plugins' directory, the routes defined by my plugin are removed (this occurs because my plugin is still located in the default directory).
A bug arises if my plugin adds an item to the admin menu : the route for this item is removed during the route reloading process.
Updated by Marius BĂLTEANU 5 months ago
Indeed, that code from setup and teardown could lead to some random failing tests, but it was changed in the current trunk, please see r22516. Could you give it a try with the new code to check if you still experiment random fails? If yes, please open a new issue because this one is already closed and we can continue the investigation there. If everything it's ok, I think we can merge the fix also to 5.1-stable branch.
Updated by Vincent Robert 5 months ago
After a quick test, it appears that the current trunk version is working fine. It would be great to integrate this fix into the current stable branch.
I will open a new issue if any new random failure tests arise as a result of these changes.
Thank you
Updated by Marius BĂLTEANU 4 months ago
Vincent Robert wrote in #note-10:
After a quick test, it appears that the current trunk version is working fine. It would be great to integrate this fix into the current stable branch.
I will open a new issue if any new random failure tests arise as a result of these changes.
Thank you
Merged to 5.1-stable in #39864.
Updated by Marius BĂLTEANU 4 months ago
- Related to Defect #39864: Backport fix of random failing integration test for plugin routes added