exercism / ruby

Exercism exercises in Ruby.
https://exercism.org/tracks/ruby
MIT License
556 stars 518 forks source link

Isogram has wrong test cases #1702

Closed kamalpanhwar closed 4 months ago

kamalpanhwar commented 4 months ago

Sorry my mistake in understanding. No action required.

https://exercism.org/tracks/ruby/exercises/isogram has some wrong test, we are testing that it is not isogram but instead of asset it is using refute!. I think refute can't be used because still we have string to compare so there is already not in the strange. It will still remain assert

Example

def test_word_with_one_duplicated_character_from_the_end_of_the_alphabet
    input = "zzyzx"
    refute Isogram.isogram?(input), "Expected false, '#{input}' is not an isogram"
  end

We are saying refute but output of this test would Expected false, '#{input}' is not an isogram so it is actually doing assert with a negative phrase. There are few more in it. so I am making a PR

github-actions[bot] commented 4 months ago

Hello. Thanks for opening an issue on Exercism 🙂

At Exercism we use our Community Forum, not GitHub issues, as the primary place for discussion. That allows maintainers and contributors from across Exercism's ecosystem to discuss your problems/ideas/suggestions without them having to subscribe to hundreds of repositories.

This issue will be automatically closed. Please use this link to copy your GitHub Issue into a new topic on the forum, where we look forward to chatting with you!

If you're interested in learning more about this auto-responder, please read this blog post.

kotp commented 4 months ago

We are saying refute but output of this test would Expected false, '#{input}' is not an isogram so it is actually doing assert with a negative phrase.

I suspect that you are misinterpreting what the second argument of the refute does. That is a message that appears if the test does not pass, rather than what is expected.

There are few more in it. so I am making a PR

I did not see it, but please do not until we can discuss it and determine if that work really needs to be done.

Looking on the forums for this discussion, but I have not yet found it. It can be kind to link to it in here, so that things are connected without having to search for things.

kamalpanhwar commented 4 months ago

Following is example taken from here:

require 'minitest/autorun'

module Isogram
  ALLOWED_CHAR = [' ', '-']
  def self.isogram?(input)
    if isValid?(input)
      "Expected true, '#{input}' is an isogram"
    else
      "Expected false, '#{input}' is not an isogram"
    end
  end

  def self.isValid?(input)
    return false if input.empty? || input.nil?

    hash = Hash.new(0)
    input.each_char { |char| hash[char] += 1 unless ALLOWED_CHAR.include? char }
    hash.values.max < 2
  end
end

class IsogramTest < Minitest::Test
  def test_word_with_one_duplicated_character_from_the_end_of_the_alphabet
    input = "zzyzx"
    refute Isogram.isogram?(input), "Expected false, '#{input}' is not an isogram"
  end
end

here zzyzx is not Isogram, so message will also come "Expected false, '#{input}' is not an isogram" but if this is reply we won't refute but we will assert

kotp commented 4 months ago

Request mentoring for the exercise, this will get resolved there.

The problem is, restricted to only this issue, that the refute is checking for a "boolean" answer, but this answer is always evaluated as truth, as the only things that evaluate as false is false and nil.