Skip to content

Conversation

@Mingun
Copy link
Contributor

@Mingun Mingun commented Aug 16, 2024

Similar to #2788, this PR moves all tests which tests top-level adjacently tagged enums (those which is defined using #[serde(tag = "t", content = "c")] attribute) into a dedicated file, removes duplicated tests and organizes tests in a logical manner. Flatten tests when adjacently tagged enum is flattened into structure I consider foremost as flatten tests, so they left untouched.

Removed tests which tests deserialization of generated tag and content field names from integers and bytes for each enum variant kind, such tests leaved only for Unit variant. This is because they are handled by the either TagOrContentFieldVisitor or TagContentOtherFieldVisitor, which can be tested only once.

Added tests includes deserialization from sequential representation, so this part of generated code becomes tested:

serde/serde_derive/src/de.rs

Lines 1680 to 1708 in 1a9ffdb

fn visit_seq<__A>(self, mut __seq: __A) -> _serde::__private::Result<Self::Value, __A::Error>
where
__A: _serde::de::SeqAccess<#delife>,
{
// Visit the first element - the tag.
match _serde::de::SeqAccess::next_element(&mut __seq)? {
_serde::__private::Some(__field) => {
// Visit the second element - the content.
match _serde::de::SeqAccess::next_element_seed(
&mut __seq,
__Seed {
field: __field,
marker: _serde::__private::PhantomData,
lifetime: _serde::__private::PhantomData,
},
)? {
_serde::__private::Some(__ret) => _serde::__private::Ok(__ret),
// There is no second element.
_serde::__private::None => {
_serde::__private::Err(_serde::de::Error::invalid_length(1, &self))
}
}
}
// There is no first element.
_serde::__private::None => {
_serde::__private::Err(_serde::de::Error::invalid_length(0, &self))
}
}
}

Mingun and others added 19 commits August 16, 2024 21:36
…ed module

Moved and renamed:
From test_annotatons
- test_adjacently_tagged_enum_bytes               => bytes
- flatten::enum_::adjacently_tagged::straitforward=> struct_with_flatten
- test_expecting_message_adjacently_tagged_enum   => expecting_message
- test_partially_untagged_adjacently_tagged_enum  => partially_untagged

From test_macros
- test_adjacently_tagged_newtype_struct           => newtype_with_newtype
- test_adjacently_tagged_enum
- test_adjacently_tagged_enum_deny_unknown_fields => deny_unknown_fields
(review this commit with "ignore whitespace changes" option on)
Change 0i32 to 1u8 so the test can be merged with the previous in the next commit
`newtype` test also integrates test with `Bytes` tag, so be like

Removed the first assert_tokens because it is the same as the first assert in the merged method
(review this commit with "ignore whitespace changes" option on)
…ed for Unit case

There is no difference what variant is deserialized so we can test only one kind of variant
(review this commit with "ignore whitespace changes" option on)
(review this commit with "ignore whitespace changes" option on)
(review this commit with "ignore whitespace changes" option on)
Copy link
Member

@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.

Thank you!

These test suite PRs are much appreciated.

@dtolnay dtolnay merged commit 31ca16d into serde-rs:master Aug 23, 2024
@Mingun Mingun deleted the adjacently-tagged-tests branch August 23, 2024 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants