Extract shared XLIFF test-document builders to remove ~130 duplicated lines

Aperta
  • Qualcosa non va?
#171 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
3/5
Tempo stimato
1-2 giorni
Idoneità per principianti
72/100
Tipo di issue
Refactoring
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
csharp
Ambito
testing

Direzione di ricerca

Inizia con LocalizationManagerTestsBase, XLiffLocalizationManagerTests e XliffLocalizationManagerWinforms nei progetti L10NSharp.Tests e L10NSharp.Windows.Forms.Tests. Confronta i metodi builder e le costanti duplicati, quindi esegui le suite di test di entrambi i progetti. Il lavoro è completato quando esiste un’unica definizione condivisa del builder XLIFF, non sono presenti membri non necessari della factory astratta, il comportamento dei test è invariato ed entrambi i progetti superano i test.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

Background

Raised by @tombogle while reviewing #147. That PR only adds tests, so the extraction was
deferred to keep it focused. The duplication itself predates #147.

Problem

L10NSharp.Windows.Forms.Tests.XliffLocalizationManagerTests is a standalone class rather
than a subclass of LocalizationManagerTestsBase<T>, because it needs
LocalizationManagerWinforms / XliffLocalizationManagerWinforms and WinForms-only hooks
such as ChooseFallbackLanguageWinforms, which the generic base knows nothing about. That
is a legitimate reason for a separate class — but it does not require duplicating the
document-building helpers, and today it does:

Method Lines Duplicated verbatim?
CreateNewDocument 14 body identical; signature differs only in formatting
CreateTransUnit 23 body identical; signature differs only in formatting
CreateTransUnitVariant 4 body identical
AddEnglishTranslation 20 byte-for-byte identical
AddChineseOfChinaTranslation 22 byte-for-byte identical
AddChineseOfTaiwanTranslation 22 byte-for-byte identical
AddChineseBareTranslation 22 byte-for-byte identical

That is 126 lines, plus the six duplicated const fields (AppId, AppName,
AppVersion, HigherVersion, LowerVersion, LiteralNewline). The practical cost is that
test data has two sources of truth: adding a language variant (as #147 did with
AddChineseBareTranslation) means pasting the same 22 lines into both projects, and the two
copies can silently drift.

Proposal

Extract the builders into a shared static class in L10NSharp.Tests, e.g.
XLiffTestDocumentBuilder, taking folderPath and appId as parameters instead of reading
instance state. Have both
L10NSharp.Tests.XLiffLocalizationManagerTests (via LocalizationManagerTestsBase<T>) and
L10NSharp.Windows.Forms.Tests.XliffLocalizationManagerTests call into it.

This is mechanically feasible today:

  • L10NSharp.Windows.Forms.Tests already has a ProjectReference to L10NSharp.Tests, and
    already reaches across it for L10NSharp.Tests.TempFolder.
  • There is no competing XLiffDocument in L10NSharp.Windows.Forms.XLiffUtils (it holds
    only XliffLocalizationManagerWinforms and XliffLocalizedStringCacheWinforms), so both
    classes are already building the same L10NSharp.XLiffUtils.XLiffDocument.

Also worth folding in

LocalizationManagerTestsBase<T> has exactly one subclass, XLiffLocalizationManagerTests.
The protected abstract CreateNewDocument / CreateTransUnit / CreateTransUnitVariant
indirection therefore has no second implementation to abstract over — the generic T plus
abstract-factory machinery is currently paying for flexibility nothing uses. Routing both
classes through a concrete XLIFF builder removes those three abstract members and leaves less
machinery than there is now, not more.

If a non-XLIFF document format is added later, reintroducing the abstraction against two real
implementations will be easier than guessing at it from one.

Out of scope

The base class's AddArabicTranslation / AddFrenchTranslation / AddSpanishTranslation /
AddRandomTranslation are not duplicated and can move or stay as convenient.

Acceptance

  • One definition of each XLIFF test-document builder.
  • L10NSharp.Tests and L10NSharp.Windows.Forms.Tests pass with no change in test behaviour.
  • Adding a new language variant requires editing one place.

Drafted with Claude Code, model Claude Opus 5 (1M context) / claude-opus-5[1m].

Lingua principale
C#
Stelle
3
Fork
10
Merge medio
2h
PR unite (30g)
2

Preparare l'ambiente

Questo progetto non fornisce container di sviluppo, Dockerfile né guida per i contributori, quindi l'ambiente è a tuo carico: parti dal suo README e consulta la nostra guida al primo contributo per i passaggi generali.

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di sillsdev/l10nsharp

Tutte le issue di sillsdev/l10nsharp

Issue simili

Altre issue su C#

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.