Skip to content

Don't require Helper from Roulette to avoid Danger failing in `Danger#validate_file_contains_plugin!`

Rémy Coutable requested to merge fix-require-that-can-cause-problem-depending-on-load-order into master

This will prevent failures when the Roulette plugin is loaded before the Helper plugin, as explained by @felipe_artur on Slack:

Looks like the problem depends on the order the plugins are loaded, if Roulette is required before the Helper class it will throw the error.

This happens because Roulette also requires Helper class with require_relative "helper" so when the check on Danger#validate_file_contains_plugin!(file) happens for Helper file no new plugins are loaded because they've been loaded before and an error is raised with Danger::Plugin.all_plugins.length == plugin_count_was returning true.

I was able to fix this on localhost with a guard clause on https://github.com/danger/danger/blob/b9104455baf578bce61b30fea1900fb525927c37/lib/danger/danger_core/plugins/dangerfile_danger_plugin.rb#L259

  def validate_file_contains_plugin!(file)
    return if file =~ /danger\/helper.rb/

    plugin_count_was = Danger::Plugin.all_plugins.length

    yield

    if Danger::Plugin.all_plugins.length == plugin_count_was
      raise("#{file} doesn't contain any valid danger plugins.")
    end
  end

I am not familiar with Danger so this is probably not a good solution and I am not sure why the loading order is different between environments.

Edited by Rémy Coutable

Merge request reports