exercism / ruby-test-runner

GNU Affero General Public License v3.0
6 stars 10 forks source link

Robot Name tests pass locally but timeout after submission #46

Open sanevillain opened 3 years ago

sanevillain commented 3 years ago
  def test_generate_all_robots
    all_names_count = 26 * 26 * 1000
    time_limit = Time.now + 60 # seconds
    seen_names = Hash.new(0)
    robots = []
    while seen_names.size < all_names_count && Time.now < time_limit
      robot = Robot.new
      seen_names[robot.name] += 1
      robots << robot
    end
    assert_equal all_names_count, robots.size, timeout_message
  end

I believe that the while loop here leads to non-deterministic behavior. The amount of robots generated is almost always different than the expected number because the loop uses the length of the seen_names keys and fails to take into consideration the total number of times a name was created

kotp commented 3 years ago

The seen names will never include duplicates, so this should be deterministic. The number of times the same name is created should be immaterial (and discarded).

Is this causing an intermittent failure for your solution?

kotp commented 3 years ago

The test does seem strange though, because there should be only unique robot names (I will read the description, I have not looked at it for years.)

SleeplessByte commented 3 years ago

This test looks correct to me. The number of names will be different if the code fails to generate all names within 60 seconds.

kotp commented 3 years ago

That is what I am seeing too @SleeplessByte . I am going to close it.

@sanevillain can comment still though. Let us know what the doubts are, and a test to maybe prove the issue, if you can.

sanevillain commented 3 years ago

Okay. This is super weird. Yesterday the problem for me with this last test case was that on every run a different number of robots was created and that number was unpredictable. What I mean by that is that the number of unique names that the while loop depends on can't be exceeded since they are used as hash keys in the condition and the number of times any given name could've been added is saved but not used. So all in all I was creating more robots in the given time than the total number of unique names.

However, today this last test case passes without a problem. I haven't changed my solution, but for some reason, it's passing every time I run it. I uploaded my solution again, but it seems to be failing on the website now. Will appreciate it if you can tell me if it's something in my code that leads to this weird behaviour

class Robot
  @@NAMES = []

  attr_reader :name

  def initialize
    self.class.generate_names if @@NAMES.empty?
    reset
  end

  def reset
    @name = @@NAMES.pop
  end

  class << self
    def forget
      @@NAMES = []
    end

    def generate_names
      ('A'..'Z').each do |a|
        ('A'..'Z').each do |b|
          (0...10).each do |c|
            (0...10).each do |d|
              (0...10).each do |e|
                @@NAMES << "#{a}#{b}#{c}#{d}#{e}"
              end
            end
          end
        end
      end

      @@NAMES.shuffle!
    end
  end
end
SleeplessByte commented 3 years ago

This solution looks like it would work (albeit the reset name being not optimal in mu opinion), without anything that's particularly causing different runs to have different results.

sanevillain commented 3 years ago

I really don't understand how this issue is already closed when every correct solution that passes the test locally times out once uploaded to the platform. (Including the example solution to the problem)

kotp commented 3 years ago

I closed it because the title of the problem states that it is non-deterministic last test case. I believed that you indicated this is not the case.

Would you want to change the title of the issue to match the problem that you are seeing, and perhaps reopen and comment?

image

If this is the message that we get every time, then we have a different issue, I think.

Can you make the adjustments that you think are needed for this issue, if it is no longer a "non-deterministic last test" and a timing problem perhaps on the server. Then we can move this issue to the test runner repository and address it there.

sanevillain commented 3 years ago

I agree that this issue has to be moved to the test runner repository.

Edited: Output of test runner on local machine

 {
    "version": 2,
    "status": "pass",
    "message": null,
    "tests": [
        {
            "name": "Can create a robot",
            "test_code": "refute_nil Robot.new",
            "status": "pass"
        },
        {
            "name": "Has name",
            "test_code": "assert_match NAME_REGEXP, Robot.new.name",
            "status": "pass"
        },
        {
            "name": "Name sticks",
            "test_code": "robot = Robot.new\noriginal_name = robot.name\nassert_equal original_name, robot.name",
            "status": "pass"
        },
        {
            "name": "Reset changes name",
            "test_code": "robot = Robot.new\noriginal_name = robot.name\nrobot.reset\nrefute_equal original_name, robot.name",
            "status": "pass"
        },
        {
            "name": "Reset before name called does not cause an error",
            "test_code": "robot = Robot.new\nrobot.reset\nassert_match NAME_REGEXP, Robot.new.name",
            "status": "pass"
        },
        {
            "name": "Reset multiple times",
            "test_code": "robot = Robot.new\nnames = []\n5.times do\n  robot.reset\n  names << robot.name\nend\n# This will probably be 5, but name uniqueness is only a requirement\n# accross multiple robots and consecutive calls to reset.\nassert names.uniq.size > 1",
            "status": "pass"
        },
        {
            "name": "Different robots have different names",
            "test_code": "refute_equal Robot.new.name, Robot.new.name",
            "status": "pass"
        },
        {
            "name": "Different name when chosen name is taken",
            "test_code": "same_seed = 1234\nKernel.srand same_seed\nrobot1 = Robot.new\nname1  = robot1.name\nKernel.srand same_seed\nrobot2 = Robot.new\nname2 = robot2.name\nrefute_equal name1, name2",
            "status": "pass"
        },
        {
            "name": "Generate all robots",
            "test_code": "all_names_count = 26 * 26 * 1000\ntime_limit = Time.now + 60 # seconds\nseen_names = Hash.new(0)\nrobots = []\nwhile seen_names.size < all_names_count && Time.now < time_limit\n  robot = Robot.new\n  seen_names[robot.name] += 1\n  robots << robot\nend\ntimeout_message = 'Timed out trying to generate all possible robots'\nassert_equal all_names_count, robots.size, timeout_message\nassert seen_names.values.all? { |count| count == 1 }, 'Some names used more than once'\nassert seen_names.keys.all? { |name| name.match(NAME_REGEXP) }, \"Not all names match #{NAME_REGEXP}\"",
            "status": "pass"
        }
    ]
}
kotp commented 3 years ago

That is excellent. Thank you @sanevillain .

kotp commented 3 years ago

I have confirmed that it runs against the test runner as well, just fine.

@ErikSchierboom can you confirm what the report is for this exercise?

ErikSchierboom commented 3 years ago

This is due to the Ruby test runner having a lower timeout than other tracks: https://github.com/exercism/tooling-invoker/blob/main/tools.json#L17 @iHiD do we want to keep that timeout?

iHiD commented 3 years ago

It times out on the test runner.

kotp commented 2 years ago

Bumping this.

iHiD commented 2 years ago

@ErikSchierboom Can individual exercises have increased timeouts yet?

ErikSchierboom commented 2 years ago

Nope