sillsdev / sillsdev/harmony

Test coverage: JsonSyncable multi-client file fan-out and malformed-file tolerance

Open
#100 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
Dominant language
C#
Stars
14
Forks
4
Avg merge
3d 21h
Merged PRs (30d)
3

Description

Follow-up from the test-suite review (batches merged as #92–#96). Existing JsonSyncable tests assert only file existence / line count and use a single client id, so the multi-client format behavior and error tolerance are untested.

Gaps

Per-client file fan-out — commits are grouped by ClientId into separate client_<id>.jsonl files, and GetSyncState aggregates heads across them. Every test uses one ClientId, so this is unexercised.

https://github.com/sillsdev/harmony/blob/03f609f11a560433ce4a7ecd598a21921b324add/src/SIL.Harmony/JsonSyncable.cs#L50-L78

Malformed / blank / truncated files — empty file yields nothing, blank lines are skipped, null deserialize is skipped. A crash mid-write leaves a truncated trailing line; no test confirms it is tolerated rather than making the whole client file unreadable.

https://github.com/sillsdev/harmony/blob/03f609f11a560433ce4a7ecd598a21921b324add/src/SIL.Harmony/JsonSyncable.cs#L116-L133

ClientIdForFile on a non-GUID filename — a stray client_*.jsonl with an unparseable id would throw from Guid.Parse inside the parallel loop; behavior is undefined by tests.

https://github.com/sillsdev/harmony/blob/03f609f11a560433ce4a7ecd598a21921b324add/src/SIL.Harmony/JsonSyncable.cs#L110-L114

Content round-trip — no test reads a commit back and verifies ChangeEntities / HybridDateTime / Metadata survived; a field-drop regression passes today.

Suggested tests

  • AddRange_MultipleClients_WritesSeparateFilesAndAggregatesSyncState
  • AddRangeThenGetChanges_RoundTripsCommitContent
  • ReadAllCommits_SkipsBlankLinesAndTruncatedTrailingLine
  • GetSyncState_IgnoresOrHandlesUnparseableClientFile

Why it matters

JSONL files are the on-disk sync format. Real projects have many clients (fan-out is load-bearing) and interrupted writes are expected in practice; today a serialization or truncation regression would go undetected.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by locating the existing JsonSyncable tests and read src/SIL.Harmony/JsonSyncable.cs at the cited file fan-out, deserialization, and ClientIdForFile sections. Add the four suggested test cases covering multiple clients, content round-tripping, blank or truncated lines, and an unparseable client filename, then run the relevant test suite and verify all cases pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
testing
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.