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

Update all test to work with YAML wire #509

Open
RobAustin opened this issue Jun 17, 2022 · 7 comments
Open

Update all test to work with YAML wire #509

RobAustin opened this issue Jun 17, 2022 · 7 comments
Assignees

Comments

@RobAustin
Copy link
Member

Update all test to work with YAML wire by using system property wire.testAsYaml, then remove TextWrire, first mark deprecated

@alamar
Copy link
Contributor

alamar commented Jul 22, 2022

First phase would be #411 to remove a separate Text wire writer, then we should consider if removing Text wire parser is feasible as well.

@RobAustin
Copy link
Member Author

RobAustin commented Sep 12, 2022

@peter-lawrey wants to remove or rather deprecate TextWire, so can you close the ticket once we get to 100%, best to list the test which still use text wire.

@alamar
Copy link
Contributor

alamar commented Sep 12, 2022

I will check which tests only use TEXT wire but not YAML. But thing is, a lot of test cases are actually covered by tests outside Wire (such as Map, Queue tests which take TEXT as input or output) and those only test TEXT wire

So we will know how much stuff is broken only when we switch. I will do that to my TextWire writer unifications and see if anything breaks.

@alamar
Copy link
Contributor

alamar commented Sep 12, 2022

TEXT wire still has to be tested since it is actively used. Moreover I'm not sure if we can deprecate TextWire completely since we may be reliant on it for reading existing text documents produced by TextWire which may not be proper YAML.

alamar pushed a commit that referenced this issue Feb 21, 2023
@alamar alamar mentioned this issue Feb 21, 2023
@alamar
Copy link
Contributor

alamar commented Feb 21, 2023

[ERROR] Tests run: 10228, Failures: 66, Errors: 27, Skipped: 30

@alamar
Copy link
Contributor

alamar commented Mar 1, 2023

YAML wire does not support deserialization of true as 1 and false as 0 - do we need it?

TextBinaryWireTest is the relevant test

@alamar
Copy link
Contributor

alamar commented Mar 1, 2023

InvalidYamWithCommonMistakesTest failures were ignored because they test for non-compliant parsing and the goal of YAML is to be compliant

alamar pushed a commit that referenced this issue May 16, 2023
Add test for YAML method writer
Fix compatibility marshallable test
Fix nested maps test
Fix some reordering tests
Fix text binary wire test
Case insensitive field name matching to match TEXT behavior
Fix type literal parsing
Fixed handling of sequences
Partially support scenario where not all fields are read
Improve test coverage of tricky scenarios
peter-lawrey pushed a commit that referenced this issue May 19, 2023
Add test for YAML method writer
Fix compatibility marshallable test
Fix nested maps test
Fix some reordering tests
Fix text binary wire test
Case insensitive field name matching to match TEXT behavior
Fix type literal parsing
Fixed handling of sequences
Partially support scenario where not all fields are read
Improve test coverage of tricky scenarios

Co-authored-by: Ilya Kaznacheev <ilya.kaznacheev@chronicle.software>
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

2 participants