Extract shared XLIFF test-document builders to remove ~130 duplicated lines
まだ誰も着手していません。
評価
- 難易度
- 3/5
- 見積もり時間
- 1〜2日
- 初心者へのやさしさ
- 72/100
調査の方向性
L10NSharp.Tests プロジェクトと L10NSharp.Windows.Forms.Tests プロジェクトの LocalizationManagerTestsBase、XLiffLocalizationManagerTests、XliffLocalizationManagerWinforms から始めます。重複している builder メソッドと定数を比較し、その後、両方のプロジェクトのテストスイートを実行します。XLIFF の共有 builder 定義が 1 つになり、不要な abstract factory メンバーがなく、テストの動作が変わらず、両方のプロジェクトが成功すれば完了です。
索引モデルが issue の本文から書いたものです。
説明
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.Testsalready has aProjectReferencetoL10NSharp.Tests, and
already reaches across it forL10NSharp.Tests.TempFolder.- There is no competing
XLiffDocumentinL10NSharp.Windows.Forms.XLiffUtils(it holds
onlyXliffLocalizationManagerWinformsandXliffLocalizedStringCacheWinforms), so both
classes are already building the sameL10NSharp.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.TestsandL10NSharp.Windows.Forms.Testspass 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].
- 主要言語
- C#
- スター
- 3
- フォーク
- 10
- 平均マージ
- 2時間
- マージ済み PR（30日）
- 2
環境構築
このプロジェクトには開発コンテナ、Dockerfile、コントリビューションガイドがありません。まず README を読み、一般的な手順ははじめてのコントリビューションガイドを参照してください。
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
sillsdev/l10nsharp のほかの issue
-
api change bug
難易度 2/5 1〜3時間 初心者へのやさしさ 82/100
-
Update .NET target from net8.0 to net10.0再び着手できるかも このイシューのプルリクエストはマージされずにクローズされました。 オープンenhancement
難易度 4/5 3〜5日 初心者へのやさしさ 52/100
-
enhancement
難易度 3/5 1〜2日 初心者へのやさしさ 68/100
-
bug
難易度 3/5 1〜2日 初心者へのやさしさ 45/100
-
bug
難易度 3/5 1〜2日 初心者へのやさしさ 35/100
sillsdev/l10nsharp の issue をすべて見る
似ている issue
-
難易度 1/5 1時間未満 初心者へのやさしさ 80/100
Unity-Technologies/unity-agent-plugin#118 ·
メンテナーはふだん 1 日以内に返信
-
The Recommended preset turns on Import Uno DSP, which only works with the Material theme対応中かも @agneszitte が今日担当しました。 オープン
難易度 1/5 1時間未満 初心者へのやさしさ 92/100
unoplatform/uno.templates#2388 ·
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 77/100
eyalgolan/statecraft-israel#2732 ·
メンテナーはふだん 1 日以内に返信
-
要トリアージ
難易度 2/5 1〜3時間 初心者へのやさしさ 64/100
メンテナーはふだん 1 日以内に返信
-
kind/chore status/draft
難易度 2/5 1〜3時間 初心者へのやさしさ 65/100
Altinn/altinn-events#1158 ·
メンテナーはふだん 1 日以内に返信