Skip to content
This repository has been archived by the owner on Dec 17, 2024. It is now read-only.

Add support for i128 / u128 #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Mingun
Copy link
Contributor

@Mingun Mingun commented Aug 5, 2023

Because MSRV is 1.56 which has support of 128-bit integers, serde_if_integer128 hack is not needed.

This is breaking change because Token enum is not marked as non_exhaustive and it is changed.

Closes #18

@Mingun
Copy link
Contributor Author

Mingun commented Aug 7, 2023

@dtolnay, is it possible to merge this (and #5 in its original, breaking manner) and release a version 2? I think, that support of 128-bit integer are good enough reason to release 2.0?

Copy link
Contributor

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

I think I would not want to do 2.x over this.

If there is a more fully featured serde testing library that people use in the meantime, that is all right with me. Serde_test is not special outside of being the crate that serde's test suite has used. But there is not a particular ecosystem value to everybody sticking to the same library as serde's test suite.

Some other things that should be evaluated for a 2.x:

  • Order-insensitive comparison, such as being able to test data structures containing HashMap/HashSet, which is not possible today.
  • Do skip_field calls need to be represented.
  • Is human_readable integrated in the right way. This is something that was tacked on substantially later than 1.0.
  • Examine all the current users of serde_test on crates.io and any lessons learned from that usage.
  • Any relevant open issues on the serde repo.

@Mingun
Copy link
Contributor Author

Mingun commented Aug 8, 2023

This support is just needed in order to make some comprehensive tests in serde test_suite. Right now I have to avoid testing support for 128-bit integer, which, given that it is supported in serde only partially, makes it much more difficult to identify places where it is absent and what can affect

Some other things that should be evaluated for a 2.x:

That's really needed to do at once in one release? It seems to me that it is enough to include that features that's ready, in order of their readiness

@Mingun
Copy link
Contributor Author

Mingun commented Aug 28, 2023

Because since serde-rs/serde#2600 the 128-bit integers are not gated anymore it would be useful to test they support and serde_test should support that for that purpose. @dtolnay, maybe you reconsider to merge this and release a new version of serde_test? It is hardly ever that anyone will notice that this change is a breaking change, because there is no reasons to match on Token (new variants is the only breaking change here), so even your non-standard understanding of crates versioning wouldn't suffer.

Because MSRV is 1.56 which has support of 128-bit integers, serde_if_integer128 hack is not needed
@jhpratt
Copy link

jhpratt commented Dec 17, 2024

Just chiming in to say that I'm surprised this isn't already a thing. There's no (easy) way to test serde other than with this crate, which is certainly the one with the widest acceptance. While I understand that there's no downside to fracturing the ecosystem for a dev dep, that doesn't mean it should happen.

@dtolnay
Copy link
Contributor

dtolnay commented Dec 17, 2024

IMO the reason a more feature-complete testing library (solving the things in #6 (review), and more actively maintained) still does not exist or cannot be regarded as competitive is 100% because this crate continues to exist and nominally be maintained, which is unfortunate. This crate involves a much smaller amount of code than most data formats, of which there are thousands, and there isn't any other reason (private API reliance of whatever) that it would need to be coupled with the serde project. I will try archiving this repo and vendoring what we need for testing serde's macros and impls into serde-rs/serde's tests directory for use as a module.

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

Successfully merging this pull request may close these issues.

Add Token::I128 and Token::U128 to serde_test
3 participants