Fix NoMethodError that is raised when the environment variable CUCUMBER_COLORS is defined

[s2k]

Description

This fixes a NoMethodError exception that is raised, when the environment variable CUCUMBER_COLORS is set, by allowing apply_custom_colors to be called right after its definition in Cucumber::Formatter::ANSIColor.

Defining the method as a class method of the module allows it to be called in the module itself.

Type of change

• Bug fix (non-breaking change which fixes an issue)

[s2k]

As a question for the reviewers: I wonder if there should be additional tests for this. While the existing tests had to be adapted to reflect the code change, I'm not (entirely) sure whether this is sufficient (given that the bug slipped through with the tests). Even though

> CUCUMBER_COLORS='passed=red,bold' cucumber --help
[…]/.rbenv/versions/3.1.2/lib/ruby/gems/3.1.0/gems/cucumber-8.0.0/lib/cucumber/formatter/ansicolor.rb:95:in `<module:ANSIColor>': undefined method `apply_custom_colors' for Cucumber::Formatter::ANSIColor:Module (NoMethodError)

      apply_custom_colors(ENV['CUCUMBER_COLORS']) if ENV['CUCUMBER_COLORS']
      ^^^^^^^^^^^^^^^^^^^

would have revealed the bug, I'm not sure this would be a good fit for another scenario in a feature file.

[aurelien-reeves]

Thanks for your PR @s2k :)

Would it be possible to keep an instance methods - which would just call the class one - in order to prevent anything to break for users who would have hacked their cucumber?

And thus we could make sure to test the two methods actually

Also there is a call to apply_custom_colors right after it has been defined. I am surprised you did not had to change that call actually, but I may have missed something with ruby here 🤔

[s2k]

Thanks for the feedback.

I had to change the definition of apply_custom_colors to be a class method, in order to be able to call it right after the definition. That's because the context is the module.

Here's an example:

module Example
  def for_instances(arg)
    p arg
  end

  def self.for_class(arg)
    p arg
  end

  begin
    for_class('Calling the class method is OK')
  rescue NoMethodError => e
    p e
  end
  
  begin
    for_instances('The instance method is not available in the class')
  rescue NoMethodError => e
    puts "That didn't work"
    puts "Message:"
    puts "*" * 40
    puts e.message
    puts "*" * 40
  end
end

Running it yields:

"Calling the class method is OK"
That didn't work
Message:
****************************************
undefined method `for_instances' for Example:Module

    for_instances('The instance method is not available in the class')
    ^^^^^^^^^^^^^
Did you mean?  for_class
****************************************

In other words: self inside a class (or module) definition is the class/module itself. To be able to call a method within the class/module definition it needs to be a class method (similar to attr_reader etc.).

I hesitate to provide an instance method, because the method is not meant to be called on an instance (but provided it anyway 🙂), so we can discuss it.

[aurelien-reeves]

Thanks for the update.

@luke-hill what is your opinion on this?

[luke-hill]

If we have a mixture of things we need (or you want to permit both the class method and the instance method variant), your better bet is simply manipulating yourself from instance to class by doing self.class.foo

outdated

[s2k]

No worries, I'll just need to revert 1560c6e, right?

[aurelien-reeves]

No worries, I'll just need to revert 1560c6e, right?

Yes, in addition to the last comments from @luke-hill

For now this can be ignored.

[s2k]

OK, as far as I can see we could now merge this PR, no?

[diabolo]

This needs releasing. The bug with CUCUMBER_COLORS is in the current release 8.0.0 7 months after it has been fixed in cucumber:main. Can we get a point release out to address this?