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

Start rewrite serde serialization tests #468

merged 11 commits into from Sep 4, 2022


Copy link

@Mingun Mingun commented Sep 1, 2022

Our serde serializer a far from ideal, but rewriting it requires a very long commit history, as I think (I already have 20 commits and I suspect that I'll double that count...). Because I'm not a big fan of long PRs, I propose that changes, that rewrites tests without changing behavior. Next tests that I plan to add will test new corrected behavior, that is why this PR could look slightly unfinished.

Added a commit that ensures, that key name is always produces valid XML name for an element

@Mingun Mingun added the serde Issues related to mapping from Rust types to XML label Sep 1, 2022
Copy link

codecov-commenter commented Sep 1, 2022

Codecov Report

Merging #468 (a5e03b5) into master (6bedf6c) will decrease coverage by 1.65%.
The diff coverage is 88.50%.

@@            Coverage Diff             @@
##           master     #468      +/-   ##
- Coverage   52.70%   51.05%   -1.66%     
  Files          29       30       +1     
  Lines       13535    13143     -392     
- Hits         7134     6710     -424     
- Misses       6401     6433      +32     
Flag Coverage Δ
unittests 51.05% <88.50%> (-1.66%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/de/ 21.05% <0.00%> (ø)
src/de/ 72.07% <0.00%> (-0.66%) ⬇️
src/de/ 85.10% <0.00%> (ø)
src/ 2.06% <0.00%> (-0.02%) ⬇️
src/ 64.84% <50.00%> (-2.12%) ⬇️
src/se/ 80.44% <90.69%> (-13.37%) ⬇️
src/se/ 92.23% <92.23%> (ø)
src/de/ 82.02% <100.00%> (-0.43%) ⬇️
src/de/ 90.63% <100.00%> (ø)
src/se/ 93.95% <100.00%> (+1.39%) ⬆️
... and 5 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link

dralley commented Sep 2, 2022

I'd like to try to not be rewriting the serde code in parallel - if there are relevant pieces of #441 which you would be willing to merge separately, let's try to do that.

Copy link
Collaborator Author

Mingun commented Sep 2, 2022

For now I see that we are practically do not intersect -- it seems that serializer implementation does not depend much on the encoding, because it always writes UTF-8 strings. I rewriting serializer completely, so it will use std::fmt::Write (instead of std::io::Write) and not deal with encodings at all Outdated
@@ -17,6 +17,7 @@
### Misc Changes

- [#468]: Content of `DeError::Unsupported` changed from `&'static str` to `Cow<'static, str>`
- [#468]: Ensure, that map keys could only types that serialized as primitives only
Copy link

@dralley dralley Sep 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure that map keys are restricted to only types that can be serialized as primitives

dralley approved these changes Sep 4, 2022
…zed as primitives

Co-authored-by: Daniel Alley <>
@Mingun Mingun merged commit f8b292b into tafia:master Sep 4, 2022
3 checks passed
@Mingun Mingun deleted the ser-tests branch September 4, 2022 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
serde Issues related to mapping from Rust types to XML
None yet

Successfully merging this pull request may close these issues.

None yet

3 participants