crichards17 / Distributed_Id_Allocator

A Rust rewrite of the ID Compressor implementation for the Microsoft Fluid Framework
0 stars 0 forks source link

Improve test coverage for Rust unit tests #84

Open crichards17 opened 1 year ago

crichards17 commented 1 year ago

Figure out how to check test coverage for Rust code

taylorsw commented 1 year ago

Current results:

Apr 30 12:55:18.623 INFO cargo_tarpaulin::report: Coverage Results: || Uncovered Lines: || distributed-id-allocator\src\compressor\persistence.rs: 17, 30-34, 120, 138, 153, 159-161 || distributed-id-allocator\src\compressor\tables\final_space.rs: 30-35, 56, 58, 91, 96 || distributed-id-allocator\src\compressor\tables\session_space.rs: 92, 109, 160, 170, 175, 277, 295, 322, 328 || distributed-id-allocator\src\compressor.rs: 213, 216, 268-274, 276, 281, 293, 376-378, 381-383, 385-386, 389, 409, 417, 446, 453, 456, 459, 512, 516, 524, 528, 581 || id-types\src\errors.rs: 46, 48-53, 55 || id-types\src\final_id.rs: 27-28 || id-types\src\local_id.rs: 45-46, 49-50, 53-54, 57-58, 61-65, 67 || id-types\src\session_id.rs: 34, 57-58, 63-64 || wasm-id-allocator\src\lib.rs: 57-58, 62-63, 86-87, 96-97, 99-102, 271, 273-276, 279, 284, 290-292, 294

86.72% coverage, 751/866 lines covered, +0.35% change in coverage

crichards17 commented 1 year ago

final_id and local_id done.

crichards17 commented 1 year ago

Current tarpaulin results analysis:

Coverage Results:|| Uncovered Lines:

|| distributed-id-allocator\src\compressor\persistence.rs: 17 // Version mismatch on deserialize. Unable to test without manual corruption of serialized compressor. Unable to test? 30-34 // Currently no tests for DeserializationError cases, and we currently never return ::MalformedInput. Needs review. 123 // The true arm for with_local_state, which is run in multiple of the deserialize tests. Flag unclear. 141 // No test for ::InvalidResumedSession. 156 // The Some() in get_tail_cluster() during deserialization. Reached during tested scenarios. Flag unclear. 162 // Same as 156, but for session's get_tail_cluster. Flag unclear.

|| distributed-id-allocator\src\compressor\tables\final_space.rs: 26-31 // Debug assert for cluster ordering. Debug. 48, 50 // Ord results for final_space binary search. Flag unclear. 83, 88 // Debug-assert (equals_test_only() ). Debug.

|| distributed-id-allocator\src\compressor\tables\session_space.rs: 60-63 // deref_cluster_mut(). This is used in finalize_range() for new cluster creation on overflow. Done: unit test added. 92 // No test the max_allocated_stable check. Done: unit test added. 107 // Potentially redundant else{}. If() branch appears to be checking the result of get_cluster_by_local(), but may be tautological. Needs review. 158, 168, 173 // equals_test_only(). Debug. 211 // Return branch for get_tail_cluster on empty cluster chain. Unreached code. 276, 294 // Ordering::greater branches for session_space binary searches. Flag unclear. 320 // None() return for get_allocated_final(). Unreached code. 326 // Fast-fail check for get_aligned_local(). Unreached code.

|| distributed-id-allocator\src\compressor.rs: 217 // finalize_range match arm for ID Range with no IDs. Flag unclear. 220 // ::MalformedIdRange not tested for. Done: unit test added. 292 // None arm for get_tail_cluster to update final_id_limit. Unreached code. 391-393 // Normalizing own local to session space Done: unit test added. 396-398, 400-401 // Normalizing eager finals to session space. Done: unit test added. 404 // Normalizing invalid local to session space. Done: unit test added. 424 // This None arm should be unreachable. No Some() result of get_cluster_by_allocated_final() should return None for get_aligned_local. Needs review. 432 // Normalizing an allocated but ungenerated eager final Done: unit test added. 465 // Here again I don't believe this is reachable. A containing_cluster result should be unable to return None for get_aligned_local(). Needs review. 472 // Believed unreachable. How can an ID that is outside of the cluster's max_local simultaneously be known to the SessionSpaceNormalizer? Needs review. 475, 478 // Untested if() arms. Done: added unit tests. 531, 535, 543, 547, 600 // Items in Test Test-only code.

|| id-types\src\errors.rs: 46, 48-53, 55 // to_string() methods. No test needed?

|| id-types\src\session_id.rs: 34 // InvalidVersionOrVariant. Add unit test. 57-58, 63-64 // ::From for String, Uuid. Unreached code.

|| wasm-id-allocator\src\lib.rs: 57-58 // get_default_cluster_capacity(). No test needed. 62-63 // get_nil_token(). Add unit test. 86-87 // get_cluster_capacity. Tested by TS. 96-97, 99-102 // set_cluster_capacity result arms. Tested by TS. 271, 273-276, 279, 284, 290-292, 294 // Test-only code.

crichards17 commented 1 year ago

"Unreached code" refers to a code branch that happens not to be hit by our tests but which I don't believe warrants removing and also don't think we need to test directly. Feel free to look these over as well and see if you agree.