test(llard): cover comparator formula loading#157
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: test(llard): cover comparator formula loading
The change adds a single e2e test-data comparator formula (testdata/kodo-e2e/formulas/pnggroup/libpng/Libpng_cmp.gox). I verified it loads and executes correctly: XGo resolves the lowercase semver.compare to the exported semver.Compare (golang.org/x/mod/semver) via cl/expr.go pkgRef(), and the formula root is the one consumed by the kodo/http e2e harnesses, so the file is actually exercised. No security, performance, or correctness (build/load) issues found.
One inline note below on the misleading comment and minor style inconsistency with the two existing sibling comparator files. Non-blocking.
| compareVer (a, b) => { | ||
| // Prefer released versions over libpng's non-semver beta tags. For example, | ||
| // v1.6.58 is valid semver while v1.7.0beta89 is not. | ||
| return semver.compare(a.Version, b.Version) |
There was a problem hiding this comment.
Comment overstates the behavior + naming inconsistent with sibling comparators.
-
The comment says this "prefer[s] released versions over libpng's non-semver beta tags," but the body just delegates to
semver.Compare(a.Version, b.Version)— there is no beta-handling logic here. The "preference" is an incidental side effect ofgolang.org/x/mod/semver: an invalid semver string canonicalizes to""and sorts below any valid version. Notev1.7.0beta89is invalid semver (a prerelease needs a hyphen,v1.7.0-beta89), so all such non-canonical tags collapse to a single "equal, lowest" bucket — they can't be ordered relative to each other, and a newerv1.7.0beta89sorts below an older releasedv1.6.58. Consider rewording to describe the actual mechanism (delegates tosemver.Compare; non-canonical tags are treated as invalid and sort lowest) rather than implying deliberate release-preference logic. -
semver.compare(lowercase) is functionally valid — XGo resolves it to the exportedsemver.Compare— but both sibling comparator files use the canonicalsemver.Compare(internal/build/testdata/pnggroup/libpng/libpng_cmp.gox:2,internal/modules/testdata/DaveGamble/cJSON/CJSON_cmp.gox:2). Prefersemver.Comparefor consistency. Those files also use 4-space indentation while this one uses tabs.
1 participant
No description provided.