Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarity on intent of good/testfile35.ion #66

Open
siler opened this issue Mar 8, 2020 · 4 comments
Open

Clarity on intent of good/testfile35.ion #66

siler opened this issue Mar 8, 2020 · 4 comments

Comments

@siler
Copy link

siler commented Mar 8, 2020

It looks like this test is attempting to cause an Ion parser to load a shared symbol table into its catalog. According to my read of the spec, however, the following top level value in the file is actually user data:

$ion_shared_symbol_table::{
  name:"test", version:1, 
  symbols:["dates","whenDate"]
}

Related section describing shown syntax: http://amzn.github.io/ion-docs/docs/symbols.html#shared-symbol-tables

This section defines the serialized form of shared symbol tables. Unlike local symbol tables, the Ion parser does not intrinsically recognize or process this data; it is up to higher-level specifications or conventions to define how shared symbol tables are communicated.

It seems like this is building towards a catalog-aware test suite that has a superset of semantics, similar to the embedded_document concept or the good/bad equivalencies files, which could be use to flex the catalog aspect of Ion.

@tgregg
Copy link
Contributor

tgregg commented Mar 9, 2020

Right. There are currently a few different test files that include shared symbol tables (which, as you say, would be interpreted as user data under the current test semantics) and/or unresolvable shared symbol table imports. I believe ion-java whitelists some of these and applies its own catalogs to them in hand-written unit tests. I agree that we need to add some semantics to ion-tests itself that dictate how to implement this across all implementations. There's is brief discussion of that here.

@m6w6
Copy link

m6w6 commented Dec 15, 2021

Sorry to jump into here, but this was the only search result for embedded_document.

There's is brief discussion of that here.

Oh lord. So is it embedded_document:: or $ion_embedded_whathever:: now?
Why are unfinished specs included in a test suite?
Please, if possible, advise how to proceed.
At least, ion-c, as of now, doesn't do anything with embedded_document::.

@tgregg
Copy link
Contributor

tgregg commented Dec 15, 2021

So is it embedded_document:: or $ion_embedded_whathever:: now?

It's still embedded_documents. The linked ion-test-driver spec includes the implicit proposal to change it to $ion_embedded_streams (because reserved Ion annotations are supposed to start with $ion_), but that proposal hasn't yet been implemented. We should probably change that documentation to refer to embedded_documents until we make the change, to avoid this confusion.

To be clear, embedded_documents are only recognized in ion-tests files. This is not part of the Ion format specification. ion-c actually does handle embedded_documents in tests; the special annotation is defined here.

@m6w6
Copy link

m6w6 commented Dec 16, 2021

Thank you for the clarification and the hint at the location within tools/ion_events lib.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants