xogroup / felicity

Javascript object constructors and sample data based on Joi schema.
Other
111 stars 9 forks source link

Felicity example for number with max constraint hangs #91

Closed chribben closed 7 years ago

chribben commented 7 years ago

Context

What are you trying to achieve or the steps to reproduce ?

The following test fails:

  describe('generating example for number with max', () => {

    it('should not hang', done => {

      const FelicityConstructor = Felicity.entityFor(Joi.number().max(1));
      const felicityInstance = new FelicityConstructor();
      const example = felicityInstance.example();

      expect(example).to.be.a.number();
      done();
    });

  })

What result did you expect ?

A successful test result

What result did you observe ?

The code hanging. Seems to be this while-loop that doesn't terminate.

WesTyler commented 7 years ago

Hey there! Thank you for raising this issue!

You actually brought up two separate things. Which is awesome, because I love fixing things and this is double the fun!

First of all, yes, you are absolutely correct. That loop is hanging for Joi.number().max(1) it ends up assigning both the default minimum and the maximum values to 1, which should obviously result in 1 being returned instead of trying to satisfy 1 < val && val < 1. I'll patch this shortly.

The second issue you brought up is the use of non-object schema as the input to Felicity.entityFor. The .entityFor method is meant to return javascript constructor functions, so there is the implicit condition that the schema should therefore be an object. In this case const felicityInstance = new FelicityConstructor() ends up returning an empty object, but felicityInstance.schema returns the Joi schema for your number. This mismatch is less than ideal. I will add a patch to throw errors when non-object schema are passed to entityFor, and will update docs to indicate this requirement.

chribben commented 7 years ago

Great work, thanks!