Test normalized paths

[jg-rp]

Similar to @f3ath's suggestion in #99, this PR changes the schema to allow result_paths or results_paths properties (I couldn't quite bring myself to use path singular for an array) and adds expected normalized paths to a few select test cases.

Currently, result_paths is only allowed when result is present, and results_paths is only allowed if results is present. I imagine if normalized paths are worth testing for a given query then the resulting values should be tested too, so testing paths alone is not currently allowed by the schema.

[f3ath]

LGTM. Will wait for approvals from @glyn and @gregsdennis.

[hiltontj]

Nice work! This is a great idea, a user had recently raised an issue on serde_json_path's normalized path generation. See hiltontj/serde_json_path#113

[f3ath]

I just ran the new tests in my implementation, all passed.

[glyn]

I think this is valuable, thanks. But I don't understand why we'd want some tests not to have normalized paths. Would it be possible to add a commit which includes all the normalized paths for all remaining tests and also have the schema mandate that result_paths is present whenever result is present and results_paths is present whenever results is present?

[jg-rp]

But I don't understand why we'd want some tests not to have normalized paths. Would it be possible to add a commit which includes all the normalized paths ..

It's possible. I was thinking 100% path test coverage would be something we'd work towards over time, over multiple commits, considering the labour involved in manually changing and reviewing all 679 test cases. I'll happily chip away at them here over the coming days if necessary.

Alternatively we could generate/validate all expected normalized paths programatically using one or more existing implementations, which would give me reasonable confidence in the correctness of the resulting tests.

If we were to generate normalized paths programmatically, it would make an alternative schema seem less daunting. For example:

(Note that I've changed selector to query in these examples to match RFC 9535 terminology.)

{
  "name": "basic, wildcard shorthand, array data",
  "query": "$.*",
  "document": ["first", "second"],
  "result": {
    "values": ["first", "second"],
    "paths": ["$[0]", "$[1]"]
  }
}

and

{
  "name": "basic, wildcard shorthand, object data",
  "selector": "$.*",
  "document": {
    "a": "A",
    "b": "B"
  },
  "results": [
    {
      "values": ["A", "B"],
      "paths": ["$['a']", "$['b']"]
    },
    {
      "values": ["B", "A"],
      "paths": ["$['b']", "$['a']"]
    }
  ]
}

[glyn]

Alternatively we could generate/validate all expected normalized paths programatically using one or more existing implementations, which would give me reasonable confidence in the correctness of the resulting tests.

Yup, I think that's the best solution. I'd be comfortable going with the paths that two implementations (from distinct implementers) agree on.

[gregsdennis]

Mine already outputs node locations. I'll see if I can batch the output.

I can work up the schema changes, too.

[f3ath]

@glyn @gregsdennis I got my version of normalized paths in #101 . We can compare the compiled cts.json to cross-check.

[jg-rp]

My last commit populates result_paths or results_paths for all valid test cases.

cts.json failing validation is expected as the schema now requires result_paths or results_paths for valid cases.

@f3ath, it looks like #101 has not updated files in tests/functions and tests/whitespace and your "basic, multiple selectors, wildcard and name" seems to have too many items in results_paths.

[f3ath]

Thanks for the review @jg-rp . I have updated #101.

[gregsdennis]

Looks like I have a bug where I'm not properly escaping chars in name selectors, but everything else passes for me.

Also, I realized that I can't fully generate the cases because I only handle a single potential ordering.

[f3ath]

Also, I realized that I can't fully generate the cases because I only handle a single potential ordering.

I used the existing results values to generate the corresponding results_paths.

[jg-rp]

@glyn, updated cts.json is now included and passes validation.

[glyn]

Thanks. Nice work @jg-rp and @f3ath.

[gregsdennis]

Just started working on my bug, and I see that it's just something I never came back to. 😆