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

Stream methods implementation #117

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

api-from-the-ion
Copy link
Contributor

@api-from-the-ion api-from-the-ion commented Nov 27, 2023

I've implemented the stream and skip methods for Yasson. See this and this. Then I saw that these things are also missing in some parser realizations of Parsson. So I just port this here, because my implementation is based on the JParser interface itself.

StreamCreator is here only because of Java 8 compatibility.

Another off-topic question: I have many PR for Yasson, and they are unreviewed. Is there anybody else who can review them? Or any other way to get them closer to be merged. Theses are the bugs and missing functions.

api-from-the-ion and others added 6 commits November 13, 2023 23:43
…created test for these methods

Signed-off-by: Anton Pinsky <anton.pinsky@ionos.com>
Signed-off-by: Anton Pinsky <anton.pinsky@ionos.com>
…corrections afterward

Signed-off-by: Anton Pinsky <anton.pinsky@ionos.com>
…P Parser in it & JsonParserImpl; higher code coverage

Signed-off-by: Anton Pinsky <anton.pinsky@ionos.com>
Signed-off-by: Anton Pinsky <anton.pinsky@ionos.com>
@api-from-the-ion
Copy link
Contributor Author

Anybody would like to take a look on this, to review and merge at the end?

@lukasj lukasj mentioned this pull request Feb 26, 2024
@lukasj
Copy link
Member

lukasj commented Feb 26, 2024

done through #124

@lukasj lukasj closed this Feb 26, 2024
@lukasj
Copy link
Member

lukasj commented Feb 27, 2024

reverted the change in #126 because b7a2278 breaks testGetValueStream test in TCK

@lukasj lukasj reopened this Feb 27, 2024
@api-from-the-ion
Copy link
Contributor Author

reverted the change in #126 because b7a2278 breaks testGetValueStream test in TCK

I'll take a look. Just have to upmegre the changes from main. How could I run the TCK tests locally? And with changes from the branch? I have to take a look at the tests to understand this.

Can I see the error messages from the failed test run somewhere?

@lukasj
Copy link
Member

lukasj commented Feb 28, 2024

TCK is available here, unzip it, edit bin/pom.xml to use the right version and run mvn test. There was "hidden" NPE for a case like new JsonParser(new StringReader("somestring").getValueStream()

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

Successfully merging this pull request may close these issues.

2 participants