[libcamera-devel] [RFC PATCH v2 05/14] test: yaml-parser: Test dictionary items ordering

[Naushir Patuck]

Hi,

On Mon, 13 Jun 2022 at 07:18, Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:

> Hi Paul,
>
> On Mon, Jun 13, 2022 at 01:22:19PM +0900, paul.elder at ideasonboard.com
> wrote:
> > Hi Laurent,
> >
> > On Sat, Jun 04, 2022 at 09:59:30PM +0300, Laurent Pinchart via
> libcamera-devel wrote:
> > > The order of items in a YAML dictionary may matter. Update the test to
> > > ensure that it is preserved. The test currently fails at the YamlParser
> >
> > My understanding is that YAML mappings are unordered [1] [2], and if
> > order in the mapping is significant, then either a sequence of mappings
> > [3] or flow mapping adjacent values [4] should be used.
>
> That's a very good point. [5] even mentions
>
> "while imposing a order on mapping keys is necessary for flattening YAML
> representations to a sequential access medium, this serialization detail
> must not be used to convey application level information."
>
> The same applies to JSON ([6]).
>
> Fixing this would require changing the syntax of the tuning files. It's
> inconvenient, but not doing so opens the door to more issues in the
> future :-S
>

Given that the IPA does require ordering and we have been lucky with
Boost preserving order in the JSON parser, I think we probably ought
to specify ordering in the config file with a specific key.  When I get a
change, I'll look to add a patch to allow this on the existing codebase,
and this series ought to "just work" after that.

Thanks,
Naush

> [1] https://yaml.org/spec/1.2.2/#mapping
> > [2] https://yaml.org/spec/1.2.2/#3221-mapping-key-order
> > [3] https://yaml.org/spec/1.2.2/#example-ordered-mappings
> > [4] https://yaml.org/spec/1.2.2/#example-flow-mapping-adjacent-values
>
> [5] https://yaml.org/spec/1.2.2/#32-information-models
> [6] https://www.json.org/json-en.html
>
> > > doesn't correctly preserve the order, this will be fixed by the next
> > > commit.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > ---
> > >  test/yaml-parser.cpp | 7 +++----
> > >  1 file changed, 3 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp
> > > index 5ff4c3236dbf..582c9caed836 100644
> > > --- a/test/yaml-parser.cpp
> > > +++ b/test/yaml-parser.cpp
> > > @@ -29,8 +29,8 @@ static const string testYaml =
> > >     "  - Mary\n"
> > >     "dictionary:\n"
> > >     "  a: 1\n"
> > > -   "  b: 2\n"
> > >     "  c: 3\n"
> > > +   "  b: 2\n"
> > >     "level1:\n"
> > >     "  level2:\n"
> > >     "    - [1, 2]\n"
> > > @@ -428,7 +428,6 @@ protected:
> > >             }
> > >
> > >             auto memeberNames = dictObj.memberNames();
> > > -           sort(memeberNames.begin(), memeberNames.end());
> > >
> > >             if (memeberNames.size() != 3) {
> > >                     cerr << "Dictionary object fail to extra member
> names" << std::endl;
> > > @@ -436,8 +435,8 @@ protected:
> > >             }
> > >
> > >             if (memeberNames[0] != "a" ||
> > > -               memeberNames[1] != "b" ||
> > > -               memeberNames[2] != "c") {
> > > +               memeberNames[1] != "c" ||
> > > +               memeberNames[2] != "b") {
> > >                     cerr << "Dictionary object fail to parse member
> names" << std::endl;
> > >                     return TestFail;
> > >             }
>
> --
> Regards,
>
> Laurent Pinchart
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220613/afe8647d/attachment-0001.htm>