Skip to content

Conversation

@Mingun
Copy link
Collaborator

@Mingun Mingun commented Aug 6, 2022

This PR fixes #435. The reason of the bug is that nested lists produces two calls of start_replay() of skipped events, but not all events should be replayed after first call.

This PR add marks that determines which events should be replayed after calling start_replay(). The mark is created when new replay scope is started (when deserialize_seq / deserialize_tuple / deserialize_tuple_struct is called) and used when start_replay() is called.

@codecov-commenter
Copy link

codecov-commenter commented Aug 6, 2022

Codecov Report

Merging #453 (656081b) into master (9ccc686) will increase coverage by 0.81%.
The diff coverage is 66.45%.

@@            Coverage Diff             @@
##           master     #453      +/-   ##
==========================================
+ Coverage   51.33%   52.15%   +0.81%     
==========================================
  Files          28       28              
  Lines       13312    13480     +168     
==========================================
+ Hits         6834     7030     +196     
+ Misses       6478     6450      -28     
Flag Coverage Δ
unittests 52.15% <66.45%> (+0.81%) ⬆️

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

Impacted Files Coverage Δ
benches/macrobenches.rs 0.00% <0.00%> (ø)
src/de/mod.rs 83.00% <93.71%> (+8.87%) ⬆️
src/de/map.rs 72.72% <100.00%> (+0.37%) ⬆️
src/de/seq.rs 93.10% <100.00%> (+1.26%) ⬆️
src/reader/buffered_reader.rs 75.63% <0.00%> (-0.51%) ⬇️
src/lib.rs 12.37% <0.00%> (+0.09%) ⬆️
src/de/var.rs 85.10% <0.00%> (+21.27%) ⬆️

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

);

// Initial conditions - both are empty
assert_eq!(de.read, vec![]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just .is_empty() is cleaner

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I would like to keep comparing with empty vector for two reasons:

  • keep uniformity of checks in the test
  • if check failed, I would see actual content of a field

@Mingun Mingun marked this pull request as draft August 7, 2022 11:12
@Mingun
Copy link
Collaborator Author

Mingun commented Aug 7, 2022

I remembered that I've implemented not all tests which I wish

@Mingun Mingun force-pushed the overlapped-with-nested-lists branch from f26f317 to a79e5d9 Compare August 7, 2022 13:50
Mingun added 2 commits August 7, 2022 19:30
failures (1):
    de::tests::skip::partial_replay

failures (14):
    seq::fixed_name::fixed_size::field_after_list::overlapped_with_nested_list
    seq::fixed_name::fixed_size::field_before_list::overlapped_with_nested_list
    seq::fixed_name::fixed_size::two_lists::overlapped_with_nested_list
    seq::fixed_name::variable_size::field_after_list::overlapped_with_nested_list
    seq::fixed_name::variable_size::field_before_list::overlapped_with_nested_list
    seq::fixed_name::variable_size::two_lists::overlapped_with_nested_list
    seq::variable_name::fixed_size::field_after_list::overlapped_with_nested_list
    seq::variable_name::fixed_size::field_before_list::overlapped_with_nested_list
    seq::variable_name::fixed_size::two_lists::choice_and_fixed::overlapped::with_nested_list_fixed_after
    seq::variable_name::fixed_size::two_lists::fixed_and_choice::overlapped::with_nested_list_fixed_after
    seq::variable_name::variable_size::field_after_list::overlapped_with_nested_list
    seq::variable_name::variable_size::field_before_list::overlapped_with_nested_list
    seq::variable_name::variable_size::two_lists::choice_and_fixed::overlapped::with_nested_list_fixed_after
    seq::variable_name::variable_size::two_lists::fixed_and_choice::overlapped::with_nested_list_fixed_after
@Mingun Mingun force-pushed the overlapped-with-nested-lists branch from a79e5d9 to 656081b Compare August 7, 2022 14:31
@Mingun Mingun marked this pull request as ready for review August 7, 2022 14:54
@dralley dralley merged commit ad27240 into tafia:master Aug 7, 2022
@Mingun Mingun deleted the overlapped-with-nested-lists branch August 7, 2022 17:41
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.

Missing field "StructuredType"

3 participants