Uploaded image for project: 'Mongoid'
  1. Mongoid
  2. MONGOID-4742

String#numeric? can return values other than true & false

    • Type: Icon: Bug Bug
    • Resolution: Fixed
    • Priority: Icon: Minor - P4 Minor - P4
    • 7.1.0.rc0, 6.4.5, 7.0.4
    • Affects Version/s: 6.1.1, 7.0.3
    • Component/s: Dev Exp
    • Labels:
      None
    • Environment:
      Docker image
      `FROM ruby:2.5.3` - DebianStretch

      I'm unsure whether Bug is the correct issue type however there are a couple of things I wanted to point out.

      So it all started with my rspec test failing

      it "doesn't override existing method" do
        expect(string.method(method).super_method).to be_nil
       end

      Where `method` was `numeric?`.

      So I investigated and found that `mongoid` gem creates a `#numeric?` method for the String objects.

      By documentation it says 

       

        # @return [ true, false ] If the string is a number.

      However by testing the code in the console it was clear that the method was not returning `true, false` but many other results.

      I added some tests to check for `true` and `false` values and I got many errors like

       

      Failure/Error: expect("-Infinity".numeric?).to eq(true)
      expected: true
       got: 0
      

      OR

       

      Failure/Error: expect("blah".numeric?).to eq(false)
      expected: false
       got: nil
      

      I'm unsure whether the documentation is wrong or the method implementation but there are many things wrong:
      1. You should not be adding methods to standard API classes as many companies might be doing that already and it might happen that they override your methods and weird things start happening
      2. The documentation / implementation are out of sync
      3. Not sure but I think tests should be checking explicitly for the result instead of doing `be_numeric` shouldn't they? I don't see a point in using the same method to check the behaviour of the method itself.
      4. `Failure/Error: expect(mongoized).to eq(expected_time)` in `
      it_behaves_like 'mongoizes to Time'` is failing due to my timezone being different. If you want developers to contribute you should be making tests compatible with any timezone
      5. There's no documentation on how to Create an issue, create a PR, run tests locally, improve code or styling

            Assignee:
            oleg.pudeyev@mongodb.com Oleg Pudeyev (Inactive)
            Reporter:
            vigenere Patryk Kotarski
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated:
              Resolved: