search

jejacks0n / teaspoon

Teaspoon: Javascript test runner for Rails. Use Selenium, BrowserStack, or PhantomJS.
1.43k stars 243 forks source link

Teaspoon::Registry#equal? is incompatible with Object#equal? #448

Closed yorickpeterse closed 8 years ago

yorickpeterse commented 8 years ago

Teaspoon::Registry defines an equal? instance method with two arguments in https://github.com/modeset/teaspoon/blob/cdf46b24676c093f7a2b0ed8e8e473dd89a47564/lib/teaspoon/registry.rb#L29. This signature is incompatible with Object#equal? which only takes a single argument. The rdoc even mentions the following:

Unlike ==, the equal? method should never be overridden by subclasses as it is
used to determine object identity (that is, a.equal?(b) if and only if a is
the same object as b):

This in turn breaks Rubinius as described in https://github.com/rubinius/rubinius/issues/3570 as we use #equal? here: https://github.com/rubinius/rubinius/blob/34af1dbb3c24aab455df12e4ce1c0568f1110d47/kernel/common/binding.rb#L14

There's only a single instance of this equal? method being used in the Teaspoon code, so renaming the method shouldn't be too much of a problem: https://github.com/modeset/teaspoon/blob/70fba0054a1155c326a120b99ff26934e97330ab/lib/teaspoon/engine.rb#L83

yorickpeterse commented 8 years ago

I can cook up a PR for this but I'd like to know if there are any suggestions as to what to rename this method to first.

jejacks0n commented 8 years ago

The info is appreciated. I would rename it to matches? or name_matches? etc, and I would merge a PR to resolve that.

jejacks0n commented 8 years ago

As a follow up, is this because teaspoon isn't working in Rubinius? If so, I appreciate your efforts here.

yorickpeterse commented 8 years ago

@jejacks0n I haven't actually ran Teaspoon itself just yet but I'll see if it's working or not.

jejacks0n commented 8 years ago

Ah, were you doing an audit for Rubinius using github search or something?

yorickpeterse commented 8 years ago

@jejacks0n No, at GitLab we might be interested in using Rubinius in the future. Since we're using Teaspoon I'm currently working on making sure everything at least works.