Skip to content

Collapse PhoneNumberUtilTest ports into one file mirroring Java order#261

Merged
rowanseymour merged 1 commit into
mainfrom
chore/collapse-phonenumberutil-tests
Jun 4, 2026
Merged

Collapse PhoneNumberUtilTest ports into one file mirroring Java order#261
rowanseymour merged 1 commit into
mainfrom
chore/collapse-phonenumberutil-tests

Conversation

@rowanseymour

Copy link
Copy Markdown
Member

What

Merge the topically-split phonenumberutil_*_test.go files (_format_, _format_dialing_, _format_natprefix_, _format_outofcountry_, _parse_basic_, _parse_extensions_, _types_, _possibility_, _normalize_, _isnumbermatch_) back into a single phonenumberutil_test.go, with the test functions ordered to mirror upstream's PhoneNumberUtilTest.java method-by-method.

Also rename the former ad-hoc test file phonenumberutil_adhoc_test.gophonenumberutil_internal_test.go and rewrite its header.

Why

This package tracks libphonenumber's Java reference implementation, and the stated goal of these test files (per SYNC.md and every file header) is to stay diffable against upstream so syncs are mechanical.

  • The topical split was only ever justified as a staging area while the old ad-hoc tests (asserted against real embedded metadata, so they go stale on every upstream refresh) were migrated to faithful ports against frozen synthetic metadata. That migration is effectively complete — every real-metadata test with a PhoneNumberUtilTest counterpart has been ported out.
  • With the staging concern gone, the split now adds drift risk (which split file does a newly-added upstream test belong in?) without benefit. A single file ordered like the Java reads top-to-bottom against PhoneNumberUtilTest.java.
  • The renamed phonenumberutil_internal_test.go holds what genuinely has no upstream PhoneNumberUtilTest counterpart: direct unit tests of internal helpers (normalizeDigits, setItalianLeadingZerosForPhoneNumber, maybeStripExtension, mergeLengths, formattingRuleHasFirstGroupOnly) and the RegexCache strictness behaviour. Its old header described a migration that's done, so it was stale/misleading.

Notes for reviewers

  • No test logic changed. The merged file contains exactly the same 120 test functions as the 11 files it replaces — verified by a name-set diff against the pre-change tree (identical) and by running the full suite (passes). The diff is large but almost entirely moves.
  • Two Go-only formatting tests with no Java sibling (TestFormatByPatternFGProdMetadata, TestFormatNationalPrefixFormattingRule) are placed next to TestFormatByPattern.
  • The adhoc → internal rename is detected by git as a rename (header-only edit).
  • gofmt/go vet clean.

Merge the topically-split phonenumberutil_*_test.go ports back into a single
phonenumberutil_test.go whose test functions are ordered to match upstream's
PhoneNumberUtilTest.java method-by-method, so the file can be diffed straight
against upstream when reconciling. The split was only ever justified as a
staging area while the ad-hoc real-metadata tests were migrated to faithful
synthetic-metadata ports; that migration is effectively complete, so the split
now adds drift risk (which file does a new upstream test belong in?) without
benefit.

Also rename the former ad-hoc test file to phonenumberutil_internal_test.go and
rewrite its header: what remains has no upstream PhoneNumberUtilTest counterpart
(direct tests of internal helpers and the RegexCache strictness behaviour), so
the "migration in progress / shrink to nothing" framing was stale.

No test logic changes: the merged file contains exactly the same 120 test
functions as the 11 files it replaces (verified by set diff), and the full
suite passes.
@rowanseymour rowanseymour merged commit aba329b into main Jun 4, 2026
6 checks passed
@rowanseymour rowanseymour deleted the chore/collapse-phonenumberutil-tests branch June 4, 2026 01:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant