Skip to content

Commit f725b02

Browse files
authored
Merge pull request #35 from Notalib/test/index-all-tables-for-findtable
test: Index every table file, not a list of extensions
2 parents 115c306 + 477bf18 commit f725b02

27 files changed

Lines changed: 36491 additions & 17 deletions

‎LibLouis.NET.Test/BrailleSpecTests.cs‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -100,9 +100,9 @@ public void SkippedEntriesMatchTheRecordedTally()
100100
var expected = new SortedDictionary<string, int>(StringComparer.Ordinal)
101101
{
102102
["test option: expected_typeform"] = 17,
103-
["test option: mode"] = 14,
103+
["test option: mode"] = 19,
104104
["test option: outputPos"] = 5067,
105-
["test option: typeform"] = 287,
105+
["test option: typeform"] = 339,
106106
["testmode: display"] = 5,
107107
["testmode: hyphenate"] = 1198,
108108
};
@@ -198,9 +198,15 @@ private static string DescribeSkipped(IReadOnlyDictionary<string, int> skipped)
198198
// the hundred-odd specs do not each re-run the same lou_findTable queries.
199199
private static readonly Lazy<Dictionary<string, string>> TableCache = new(() =>
200200
{
201-
LibLouis.Instance.IndexTables(
202-
Directory.EnumerateFiles(TableDirectory)
203-
.Where(f => Path.GetExtension(f) is ".ctb" or ".utb" or ".uti" or ".dis" or ".cti" or ".dic"));
201+
// Every file in the directory, not a list of extensions. liblouis itself indexes whatever
202+
// LOUIS_TABLEPATH contains without looking at the name (metadata.c, indexTablePath ->
203+
// listDir), and lou_indexTables already skips a file it finds no metadata in, so a filter
204+
// here can only ever be wrong in one direction: it drops tables upstream would have
205+
// indexed. That is what it did - the extension list predated the .tbl files, so all 55 of
206+
// them were invisible, and .tbl is exactly where upstream now puts the metadata that
207+
// lou_findTable matches on. Queries that should have selected one either found nothing or
208+
// settled for a worse match from another language.
209+
LibLouis.Instance.IndexTables(Directory.EnumerateFiles(TableDirectory));
204210

205211
var resolved = new Dictionary<string, string>(StringComparer.Ordinal);
206212

‎LibLouis.NET.Test/braille-specs/README.md‎

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
Copied verbatim from `upstream/liblouis-<version>/tests/braille-specs/`. Re-copy when the upstream
44
version is bumped; the diff is the set of expectations that changed.
55

6-
100 specs are here, from liblouis 3.39.0, covering roughly 60 languages. Upstream ships 160.
6+
125 specs are here, from liblouis 3.39.0, covering roughly 70 languages. Upstream ships 160.
77

88
The selection below was made against 3.33.0. Specs upstream has added since have not been tried yet:
99
`en-g3-dictionary.yaml`, `en-g3.yaml`, `en-gb-comp8.yaml`, `et_harness.yaml`, `it.yaml`,
@@ -17,26 +17,35 @@ The selection below was made against 3.33.0. Specs upstream has added since have
1717
200,000+ cases each, around 800,000 in total. They would dominate every run for little extra signal.
1818
Add them behind a switch if wanted.
1919

20-
### 39 specs that do not yet pass
20+
### 14 specs that do not yet pass
2121

2222
Not excluded because they are wrong — excluded because nobody has worked out yet whether the
2323
disagreement is this harness or the wrapper, and a red suite that is expected to be red is worse
2424
than a smaller green one. They divide into three groups.
2525

26-
**27 fail on table resolution, and are probably one root cause rather than 27.** 20 report no table
27-
matching a query, 7 resolve to a table other than the one the spec's `__assert-match` names. The
28-
queries themselves look well formed — `language:bn grade:1` — so the likely cause is which tables
29-
reach `lou_indexTables`, or which of them liblouis manages to analyse. Worth starting here: it is
30-
the largest group and the cheapest to be wrong about.
26+
**2 fail on a construct this harness reads differently from upstream.** Both resolve their tables
27+
correctly and disagree on a handful of cases each; in both the harness is the one that is wrong.
3128

3229
```
33-
bn.yaml cs_harness.yaml eo-g1_harness.yaml es-g0-g1.yaml ethio-g1_harness.yaml hi_harness.yaml
34-
hr-8dots_harness.yaml hu-hu-g1_braille_input_backward.yaml hu-hu-g1_braille_input_forward.yaml
35-
hu-hu-g1_dictionary_numbers.yaml hu-hu-g1_harness.yaml kmr.yaml litdigits6Dots_backward.yaml
36-
lv_harness.yaml ml.yaml nl-g0_harness.yaml pa.yaml
37-
pl-g1.yaml sl-g1.yaml tr.yaml
30+
en-us-g2.yaml 6/20
31+
es-g0-g1.yaml 8/992
3832
```
3933

34+
`en-us-g2.yaml` has a `flags: {testmode: bothDirections}` block followed by three bare `tests:`
35+
blocks. This harness carries the flags forward; upstream does not. `lou_checkyaml.c` declares
36+
`int testmode = MODE_DEFAULT;` *inside* the loop over the `flags`/`tests` keys, so a `tests:` block
37+
with no `flags:` of its own runs forward. The six cases that fail here are ones the harness runs
38+
backward and upstream runs forward — back-translating five identical cells and expecting five
39+
different bullet characters cannot pass in any case.
40+
41+
`es-g0-g1.yaml` uses `'\n'`, `'\r'` and `'\s'`. `BrailleSpec.Unescape` handles only `\xNNNN`,
42+
`\yNNNN`, `\uNNNN`, `\\` and `\"`, so those three reach liblouis as a literal backslash plus a
43+
letter and translate to two cells. liblouis's own parser (`parseCharsInternal` in
44+
compileTranslationTable.c, reached from `lou_checkyaml` via `_lou_extParseChars`) also takes
45+
`\e \f \n \r \s \t \v \w` and the 8-digit `\ZNNNNNNNN`. Worth fixing with care: `\s` in particular
46+
is used across the specs, so completing the list changes the input of cases that pass today —
47+
`spaces.yaml`, in the next group, is the obvious one to re-check afterwards.
48+
4049
**10 disagree on translation output**, a fraction of their cases each. These are the interesting
4150
ones: either a per-case option this harness does not model, or a real difference between the wrapper
4251
and upstream.
@@ -59,8 +68,24 @@ zh-tw.yaml 2/20
5968
unindented line (`- [` then `disingenuous, ...]`), which libyaml accepts and YamlDotNet 18.1.0
6069
throws on.
6170

71+
### What used to be here: the 27 table-resolution failures
72+
73+
Gone as of the fix in `BrailleSpecTests.TableCache`, recorded because the shape of the mistake is
74+
worth not repeating. The harness indexed `tables/` through an extension allow-list that predated
75+
upstream's `.tbl` files, so all 55 of them were invisible to `lou_indexTables` — and `.tbl` is
76+
exactly where upstream keeps the `#+language:` / `#+grade:` / `#+type:` metadata `lou_findTable`
77+
matches on. 20 queries therefore matched nothing and 7 settled for a worse match from another
78+
language: `language:ar grade:1` resolved to `he-IL.utb`, `language:en region:en-US grade:2` to
79+
`en-ueb-g2.ctb`. liblouis applies no filter of its own — `indexTablePath` in metadata.c hands it
80+
every file on `LOUIS_TABLEPATH` and drops the ones with no metadata — so a filter here could only
81+
ever be wrong in one direction. 25 of the 27 passed as soon as it was removed.
82+
6283
## Restoring one
6384

6485
Copy it back from the upstream tarball and run the suite. Note that liblouis caches compiled tables
6586
process-wide, so when bisecting a disagreement, use a cold process per variant — otherwise the
6687
result depends on which test compiled a given table first.
88+
89+
`CopyToOutputDirectory` never deletes, so a `bin/` from an older liblouis keeps tables upstream has
90+
since removed, and those get indexed alongside the current set. If a query resolves to a table that
91+
is not in `LibLouis.NET.Tables/tables`, delete `bin/` and build again.

0 commit comments

Comments
 (0)