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.

Sign up for GitHub

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

Jump to bottom

Loader Bugs for Integer Loading #218

Open
almann opened this issue Apr 25, 2021 · 1 comment
Open

Loader Bugs for Integer Loading #218

almann opened this issue Apr 25, 2021 · 1 comment
Labels
bug Something isn't working

Comments

@almann
Copy link
Contributor

almann commented Apr 25, 2021

As part of implementing #214, the following are problematic and appear to be a result in how we're integrating with ion-c.
We should adjust that binding code to make sure we can support these. My suspicion is that there is an asymmetry with binary versus text and it may be an ion-c bug, but more investigation is necessary.

    // these appear to have a problem specific to how we're calling ion-c and should be investigated
    "ion-tests/iontestdata/good/equivs/intsLargeNegative1.10n",
    "ion-tests/iontestdata/good/equivs/intsLargePositive1.10n",
    "ion-tests/iontestdata/good/intLongMaxValuePlusOne.10n",
    "ion-tests/iontestdata/good/item1.10n",
    "ion-tests/iontestdata/good/subfieldVarInt.ion",
    "ion-tests/iontestdata/good/typecodes/T2.10n",
    "ion-tests/iontestdata/good/typecodes/T3.10n",
@almann almann added the bug Something isn't working label Apr 25, 2021
almann added a commit to almann/ion-rust that referenced this issue Apr 25, 2021
@almann
Adds ion-tests test file integration for Loader.
693ffa5 
* Adds `test-generator` create dev dependency to get test per file.
* Adds a test suite with `good`/`bad` files with skip lists.
  - `equivs` and `non-equivs` are also supported with
    `embedded_document` parsing.
* Adds `Loader::load_all` convenience method.
* Adds doc test example in `loader` module docs and fixes the iterator
  example.

Additional notes:
* Added amazon-ion#216 as a result of non-equivs testing and it definitely exposes
  a weakness in our unit tests and bug in our `PartialEq` for `...Struct`.
* Added amazon-ion#218 to track a particular set ion-c integration issues around integer
  sizes and binary reader.
* Added amazon-ion#219 to track adding support for unknown text symbol tokens in `Loader`.
* Added amazon-ion#220 to track adding `IonEq`.
* Added amazon-ion/ion-c#235 as these tests run into problems with negative
  binary integer syntax (but not hex or decimal), indicating a potential
  issue there.

Resolves amazon-ion#214.
@almann almann mentioned this issue Apr 25, 2021
Merged
almann added a commit that referenced this issue Apr 26, 2021
@almann
Adds ion-tests test file integration for Loader. (#217)
1d1d0d8 
* Adds `test-generator` create dev dependency to get test per file.
* Adds a test suite with `good`/`bad` files with skip lists.
  - `equivs` and `non-equivs` are also supported with
    `embedded_document` parsing.
* Adds `Loader::load_all` convenience method.
* Adds doc test example in `loader` module docs and fixes the iterator
  example.

Additional notes:
* Added #216 as a result of non-equivs testing and it definitely exposes
  a weakness in our unit tests and bug in our `PartialEq` for `...Struct`.
* Added #218 to track a particular set ion-c integration issues around integer
  sizes and binary reader.
* Added #219 to track adding support for unknown text symbol tokens in `Loader`.
* Added #220 to track adding `IonEq`.
* Added amazon-ion/ion-c#235 as these tests run into problems with negative
  binary integer syntax (but not hex or decimal), indicating a potential
  issue there.
* Added frehberg/test-generator#12 to capture the empty glob issue.

Resolves #214.
@desaikd desaikd mentioned this issue Jun 2, 2021
Merged
@desaikd
Copy link
Contributor

desaikd commented Jun 2, 2021

Most of the tests mentioned above in description passes now with #276 change.

Some tests are still left for investigation:

 "ion-tests/iontestdata/good/item1.10n",
 "ion-tests/iontestdata/good/subfieldVarInt.ion",

@desaikd desaikd reopened this Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@almann @desaikd