Consolidate integration tests for EventCacheStore

[mgoldenberg]

Overview

There are a number of tests in the SQLite and IndexedDB implementations of the EventCacheStore which are general enough to be moved into the EventCacheStoreIntegrationTests trait. This pull request consolidates these tests to the extent possible.

Changes

Consolidating Tests

For existing test functions in matrix-sdk-sqlite::event_cache_store and matrix-sdk-indexeddb::event_cache_store, one of the following actions were taken.

• None - the function tests only implementation-specific details and consequently could not be converted to a generalized integration test.
• Copied - the function tests some implementation-specific details, but primarily interacts with the EventCacheStore trait, so it was converted into a generalized integration test and copied into the EventCacheStoreIntegrationTests trait.
• Moved - the function does not test any implementation-specific details and only interacts with the EventCacheStore trait, so it was moved into the EventCacheStoreIntegrationTests trait.
• Removed - the function tests properties that are already tested by other functions.

For the action taken for each existing function, see the tables at the end of this pull request.

Behavioral Inconsistencies

The process of converting existing tests into integration tests revealed that there were some behavioral differences between MemoryStore's implementation of EventCacheStore and other existing implementations - i.e., SQLiteEventCacheStore and IndexeddbEventCacheStore.

1. MemoryStore does not operate transactionally - i.e., a failure in a list of updates does not rollback previous updates in that list.
   • This was handled by keeping tests of transactional operations in matrix_sdk_sqlite and matrix_sdk_indexeddb and documenting that MemoryStore does not operate transactionally.
2. MemoryStore does not allow referencing a chunk before it exists in the store - e.g., Update::NewItemsChunk::next may not provide a chunk identifier that is not in the store.
   • This was handled by updating the logic of SQLiteEventCacheStore and IndexeddbEventCacheStore so that they too would not allow referencing a chunk before it exists in the store.
   • A new integration test was added to ensure this functionality is working properly - i.e., test_linked_chunk_exists_before_referenced.
3. MemoryStore allows duplicate chunk identifiers - i.e., one can add a new chunk and assign it a chunk identifier that already exists in the store.
   • This issue is outlined in MemoryStore allows duplicate ChunkIdentifier's #6095 and will likely be handled in a separate pull request.

Further Work

Since starting work on this pull request, some changes have been introduced to the SQLiteEventCacheStore (see #6065). These changes ensure that the SQLiteEventCacheStore can store the same event in multiple chunks and, furthermore, add a test to confirm this behavior.

These changes have not yet been applied to IndexeddbEventCacheStore (see #6094), so corresponding tests for these new behaviors could not be turned into integration tests yet.

Tables

matrix-sdk-sqlite

Test	Action
test_pool_size	None
test_linked_chunk_new_items_chunk	Removed
test_linked_chunk_new_gap_chunk	Removed
test_linked_chunk_replace_item	Moved
test_linked_chunk_remove_chunk	Copied
test_linked_chunk_push_items	Removed
test_linked_chunk_remove_item	Copied
test_linked_chunk_detach_last_items	Moved
test_linked_chunk_start_end_reattach_items	Moved
test_linked_chunk_clear	Copied
test_linked_chunk_multiple_rooms	Moved
test_linked_chunk_update_is_a_transaction	None
test_filer_duplicate_events_no_events	Moved
test_load_last_chunk	Moved
test_load_last_chunk_with_a_cycle	Moved
test_load_previous_chunk	Moved
test_event_chunks_allows_same_event_in_room_and_thread	None

matrix-sdk-indexeddb

Test	Action
test_linked_chunk_new_items_chunk	Removed
test_add_gap_chunk_and_delete_immediately	Removed
test_linked_chunk_new_gap_chunk	Removed
test_linked_chunk_replace_item	Moved
test_linked_chunk_remove_chunk	Copied
test_linked_chunk_push_items	Removed
test_linked_chunk_remove_item	Copied
test_linked_chunk_detach_last_items	Moved
test_linked_chunk_start_end_reattach_items	Moved
test_linked_chunk_clear	Copied
test_linked_chunk_multiple_rooms	Moved
test_linked_chunk_update_is_a_transaction	None
test_load_last_chunk	Moved
test_load_previous_chunk	Moved

---

Closes #5342.

• Public API changes documented in changelogs (optional)

Signed-off-by: Michael Goldenberg [email protected]

[codspeed-hq]

Merging this PR will not alter performance

✅ 50 untouched benchmarks

---

_{Comparing mgoldenberg:consolidate-event-cache-store-tests (f2abc55) with main (40c6c33)}

[codecov]

Codecov Report

❌ Patch coverage is 99.30556% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.82%. Comparing base (2f7887d) to head (f2abc55).
⚠️ Report is 13 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
crates/matrix-sdk-sqlite/src/event_cache_store.rs	88.23%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6061      +/-   ##
==========================================
- Coverage   89.84%   89.82%   -0.02%     
==========================================
  Files         362      362              
  Lines      100433    99931     -502     
  Branches   100433    99931     -502     
==========================================
- Hits        90230    89760     -470     
+ Misses       6678     6668      -10     
+ Partials     3525     3503      -22     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[andybalaam]

This looks good to me, but I'd like @Hywan to take a quick look please!