feature: New bazel test for config and dictionaries - #9
Merged
frankslin merged 4 commits intoJan 3, 2026
Conversation
- Switch all tests (C++ CLI, Python, Node) to consume `testcases.json` and drop `.in`/`.ans` dependencies; keep filegroup for the JSON. - Prune TWPhrases sub-dictionary artifacts and align DictionaryTest to current generated dict set. - Add rapidjson dep/path for CLI test, refresh_assets script fixes, and keep Bazel Python toolchain note.
There was a problem hiding this comment.
Pull request overview
This PR modernizes the test infrastructure by consolidating all test cases into a single testcases.json file, replacing the legacy .in/.ans file pairs. The change improves maintainability by establishing a single source of truth for test data across CLI, Python, and Node.js test suites.
Key Changes:
- Unified test data format using
testcases.jsonwith structured test cases containing input, expected outputs per configuration, and unique IDs - Updated all test runners (C++, Python, Node.js) to consume JSON-based test cases instead of file-based inputs
- Streamlined dictionary build to exclude intermediate phrase components from standalone binary outputs
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/testcases/testcases.json | New consolidated JSON file containing all test cases with inputs and expected outputs per config |
| test/testcases/.in, test/testcases/.ans | Removed legacy test input/answer files (migrated to JSON) |
| test/testcases/BUILD.bazel | Updated to reference testcases.json instead of globbing .in/.ans files |
| test/CommandLineConvertTest.cpp | Refactored to parse testcases.json and dynamically generate test cases |
| test/BUILD.bazel | Added rapidjson dependency for JSON parsing |
| test/CMakeLists.txt | Added rapidjson include path for test compilation |
| python/tests/test_opencc.py | Migrated from glob-based file reading to JSON-based test iteration |
| node/test.js | Refactored to read testcases.json and iterate over cases instead of hardcoded config list |
| data/dictionary/DictionaryTest.cpp | Updated dictionary list to exclude removed TWPhrases component files |
| data/dictionary/BUILD.bazel | Added PHRASE_PARTS exclusion to prevent standalone .ocd2 generation for merge components |
| data/config/ConfigDictValidationTest.cpp | New end-to-end validation test for configs against testcases.json |
| data/config/BUILD.bazel | Added cc_test target for config validation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Rename and guard streams in CommandLineConvertTest; ensure input file opens and normalize CRLF. - Fix node test promise handling to propagate errors correctly. - Mark ConfigDictValidationTest as Bazel-only to skip CMake builds.
frankslin
deleted the
feature/new-bazel-test-for-config-and-dictionaries
branch
January 3, 2026 03:06
frankslin
added a commit
that referenced
this pull request
Jan 3, 2026
## Summary
- add a `//data/config:config_dict_validation_test` to test dictionaries and configs against a `testcases.json` file
- switch all CLI/Python/Node tests to consume `testcases.json` as the single source of truth; drop `.in/.ans` dependencies and adjust Bazel/CMake wiring
- streamline dictionary build outputs (no standalone `TWPhrases{IT,Name,Other}.ocd2`) and align DictionaryTest with the actual generated dict set
- add maintenance helpers (refresh_assets.sh cleanup and fix, rapidjson dep/path for CLI test) and keep wasm assets in sync via `testcases.json`
## Testing
- bazel test //data/dictionary:dictionary_test
- bazel test //test:command_line_converter_test
- bazel test //python/tests:test_opencc
- node/test.js (sync/async/promise) using updated testcases.json
----
* feature: add a new ConfigDictValidationTest.cpp to be executed in bazel
* Changeover to JSON-based testcases and clean dictionary outputs
- Switch all tests (C++ CLI, Python, Node) to consume `testcases.json` and drop `.in`/`.ans` dependencies; keep filegroup for the JSON.
- Prune TWPhrases sub-dictionary artifacts and align DictionaryTest to current generated dict set.
- Add rapidjson dep/path for CLI test, refresh_assets script fixes, and keep Bazel Python toolchain note.
* Normalize CommandLineConvertTest for CRLF comparisons on Windows
* Address review feedback for tests and Bazel-only validation
- Rename and guard streams in CommandLineConvertTest; ensure input file opens and normalize CRLF.
- Fix node test promise handling to propagate errors correctly.
- Mark ConfigDictValidationTest as Bazel-only to skip CMake builds.
frankslin
pushed a commit
that referenced
this pull request
Jul 24, 2026
Correctness hardening, decoder unification, and API cleanup from review; also extends character-level filtering to the single-dictionary fast path, which the review identified as the largest remaining win. Detailed Changes: - **Shared UTF-8 decoding (review #1-#3)**: - The scanner's 3-byte branch is now explicitly guarded (charLength == 2 || charLength == 3) instead of relying on the implicit invariant that length 1 cannot reach it. - internal::DecodeCodePoint23() is the single decoder used by both the skip-table builder and the scanner, so a key's first character always maps to the bit the scanner tests; the incorrect comment about invalid continuation bytes is replaced with the byte-consumption equivalence argument. - New invariant test: for every dictionary key, SkipUnmatchable() must return 0 on both the fast path and the table path. - **Utf8SkipTable invariants (review #4, #5)**: - Character-level mode is now derived from bmpCandidates being non-empty; the separate charLevel flag is gone. - IDS operator marking iterates UTF8Util::kFirst/kLast IdeographicDescriptionOperator and consults IdeographicDescriptionOperatorArity(), replacing the hard-coded 0xE2 and 0x2FF0..0x2FFF literals; the constants live next to the arity switch with a sync note. - **API and ABI (review #6-#8)**: - PrefixMatch.hpp no longer includes Utf8SkipScan.hpp (and thus no SIMD intrinsic headers); the skip table lives in the opaque Tables pimpl for both paths and sizeof(PrefixMatch) is back to its previous value. Note PrefixMatch.hpp and Utf8SkipScan.hpp are in LIBOPENCC_PRIVATE_HEADERS and are not installed. - dynamic_cast<MarisaDict> is replaced by a virtual Dict::EnumerateKeys(cb) with a GetLexicon()-walking default; MarisaDict overrides it with a trie walk (using the materialized lexicon when one already exists) and DictGroup recurses into children, restoring group handling on the fast path. prefix_match_lib no longer depends on marisa_dict_lib. OPENCC_ABI_VERSION bumped 1.4 -> 1.5 for the new virtual. - **Fast-path character-level filtering and caching (review #9, #10)**: - Both paths now build the same character-level skip table via EnumerateKeys, and fast-path tables go through the existing Tables cache (with a distinct key prefix). Measured convert-phase speedups vs the byte-level baseline on the 1.9MB zuozhuan corpus: t2tw 5.3x, tw2sp 2.9x, t2s 1.7x, s2twp 1.3x, s2tw 1.2x, s2t unchanged; load times unchanged within a few ms. - **Scan details (review #11, #12)**: - SWAR fallback resolves the mismatch byte with ctz on little-endian instead of rescanning; 32-bit ARM NEON now takes the vector path. - **Tests (review section 4)**: - Differential fuzz test: 300 random inputs mixing dictionary keys, CJK, IDS operators, and raw invalid/truncated bytes, comparing Conversion::Convert against a per-character reference loop (same output or same exception). - 4-byte character coverage (lead-byte filtering with and without 4-byte keys), enumeration-failure fallback that clears a partially built bitmap mid-walk, and unsigned literals in skip-scan comparisons. Benchmarks are reproducible with src/tools/SpeedBenchmark.cpp or the CLI's --measured_result flag; corpora were test/benchmark/zuozhuan.txt and its s2t-converted Traditional variant, plus test/golden/input/ us_constitution_zhs.txt repeated 100x.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
//data/config:config_dict_validation_testto test dictionaries and configs against atestcases.jsonfiletestcases.jsonas the single source of truth; drop.in/.ansdependencies and adjust Bazel/CMake wiringTWPhrases{IT,Name,Other}.ocd2) and align DictionaryTest with the actual generated dict settestcases.jsonTesting