search

square / fdoc

Documentation format and verification
Other
380 stars 59 forks source link

Test suite passing? #33

Open harlow opened 11 years ago

harlow commented 11 years ago

I tried running the specs from master and I'm seeing a little red. Maybe one of my gem versions is incorrect?

Failures:

  1) Fdoc::EndpointScaffold#consume_response for succesful responses produces a valid JSON schema for the response
     Failure/Error: JSON::Validator.validate!(subject.response_parameters, response_params).should be_true
     NoMethodError:
       undefined method `each' for "???":String
     # ./spec/fdoc/endpoint_scaffold_spec.rb:223:in `block (4 levels) in <top (required)>'

  2) Fdoc::Endpoint#consume_request complex examples is successful
     Failure/Error: subject { endpoint.consume_request(params) }
     NoMethodError:
       undefined method `each' for false:FalseClass
     # ./lib/fdoc/endpoint.rb:19:in `consume_request'
     # ./spec/fdoc/endpoint_spec.rb:37:in `block (3 levels) in <top (required)>'
     # ./spec/fdoc/endpoint_spec.rb:113:in `block (4 levels) in <top (required)>'

  3) Fdoc::Endpoint#consume_request complex examples with no optional keys is successful
     Failure/Error: subject { endpoint.consume_request(params) }
     NoMethodError:
       undefined method `each' for true:TrueClass
     # ./lib/fdoc/endpoint.rb:19:in `consume_request'
     # ./spec/fdoc/endpoint_spec.rb:37:in `block (3 levels) in <top (required)>'
     # ./spec/fdoc/endpoint_spec.rb:124:in `block (5 levels) in <top (required)>'

Let me know if anything jumps out to you here. I'd like to get it green before creating any PRs.

zachmargolis commented 11 years ago

Nothing jumps out. I will try to bisesct to find the source.

In the meantime, please go ahead and start your branch. If there are merge conflicts I can try to help resolve them.

zachmargolis commented 11 years ago

Ok so I think this is definitely a gem version issue.

I made some small changes based on rspec changes (subject, expect syntax changes), going to move those to master soon.

For reference, I ran a bundle list on my machine and got this, running 1.9.3. I'm not 100% positive about harcoding each of those dependencies, maybe big ones like JSON schema?

harlow commented 11 years ago

Digging into this a little deeper and following execution path into json-schema it looks like there is an issue with looking at the value of required in the schema and then preforming an each on it.

From json-schema's README required should be an array: https://github.com/hoxworth/json-schema#validate-a-json-object-against-a-json-schema-object-while-also-validating-the-schema-itself

When my specs are running there is true/false being passed to json-schema instead of an array.

Sites/opensource/fdoc(hw-spec-updates!) $ bundle exec rspec ./spec/fdoc/endpoint_spec.rb:112
Run options: include {:locations=>{"./spec/fdoc/endpoint_spec.rb"=>[112]}}

From: /Users/harlow/Sites/opensource/json-schema/lib/json-schema/attributes/required.rb @ line 6 JSON::Schema::RequiredAttribute.validate:

     4: def self.validate(current_schema, data, fragments, processor, validator, options = {})
     5:   if data.is_a?(Hash)
 =>  6:     binding.pry
     7:     current_schema.schema['required'].each do |property,property_schema|
     8:       if !data.has_key?(property)
     9:         prop_defaults = options[:insert_defaults] &&
    10:                         current_schema.schema['properties'] &&
    11:                         current_schema.schema['properties'][property] &&
    12:                         !current_schema.schema['properties'][property]["default"].nil? &&
    13:                         !current_schema.schema['properties'][property]["readonly"]
    14:         if !prop_defaults
    15:           message = "The property '#{build_fragment(fragments)}' did not contain a required property of '#{property}'"
    16:           validation_error(processor, message, fragments, current_schema, self, options[:record_errors])
    17:         end
    18:       end
    19:     end
    20:   end
    21: end

[1] pry(JSON::Schema::RequiredAttribute)> current_schema.schema
=> {"required"=>false,
 "type"=>"object",
 "description"=>"It's a bug",
 "properties"=>
  {"required_param"=>
    {"required"=>true, "description"=>"It's required", "type"=>"string"},
   "optional_param"=>
    {"required"=>false, "description"=>"It's optional", "type"=>"string"}},
 "additionalProperties"=>false}
[2] pry(JSON::Schema::RequiredAttribute)> current_schema.schema['required']
=> false
[3] pry(JSON::Schema::RequiredAttribute)> exit
F

Failures:

  1) Fdoc::Endpoint#consume_request complex examples is successful
     Failure/Error: subject { endpoint.consume_request(params) }
     NoMethodError:
       undefined method `each' for false:FalseClass
zachmargolis commented 11 years ago

Yes, you're right.

Turns out pinning json-schema < '2.0.0' fixes this.

aew commented 11 years ago

Pinning json-schema is a temporary fix, correct? It seems that json-schema draft 4 changes the syntax for require (e.g., now it's specified as an array of fields on the object rather than a boolean, and the fields within the object can omit the required property).

Here's the new test: https://github.com/hoxworth/json-schema/blob/master/test/test_jsonschema_draft4.rb#L199

Here's the relevant commit: https://github.com/hoxworth/json-schema/commit/3e8b4a2ce2a9298540be8e22514764418049d482#test/test_jsonschema_draft4.rb

Thoughts on upgrading this?

zachmargolis commented 11 years ago

@aew yeah that makes sense. If we upgrade fdoc to use the new JSON schema format, we should probably provide a migration script for .fdoc files to the new style.