zalando-incubator / spark-json-schema

JSON schema parser for Apache Spark
MIT License
81 stars 43 forks source link

#39 Updating spark-sql version and testing references #40

Open charlyraffellini opened 5 years ago

charlyraffellini commented 5 years ago

Updating spark-sql version 2.0.1 => 2.4.3

Adding two test cases for references of multiple types. I found not obvious from the tests cases how the conversion of references like the one below will be. So I added two test cases.

          {
            "definitions": {
              "address": {
                "type": ["object", "array"],
                "properties": {
                  "street_address": { "type": "string" },
                  "city":           { "type": "string" },
                  "state":          { "type": "string" }
                }
              }
            },
            "type": "object",
            "properties": {
              "billing_address": { "$ref": "#/definitions/address" }
            }
          }
hesserp commented 3 years ago

Hi @charlyraffellini, sorry for no reply for so long time. The travis CI issue for the automatic PR checks should be fixed in the current master.

I am not sure about the additional value of your test. I think multiple types and strict typing behavior is elaborated enough here: https://github.com/zalando-incubator/spark-json-schema/blob/master/src/test/scala/org/zalando/spark/jsonschema/SchemaConverterTest.scala#L395 https://github.com/zalando-incubator/spark-json-schema/blob/master/src/test/scala/org/zalando/spark/jsonschema/SchemaConverterTest.scala#L414

and resolving references here: https://github.com/zalando-incubator/spark-json-schema/blob/master/src/test/scala/org/zalando/spark/jsonschema/SchemaConverterTest.scala#L84

I think by mixing these two aspects your intent gets unclear. Could you elaborate which aspect you want to show exactly?