Skip to content

fix(flint-py): point run_full_eval at the moved fixture corpus - #81

Open
zl190 wants to merge 1 commit into
microsoft:devfrom
zl190:fix/eval-fixtures-path
Open

fix(flint-py): point run_full_eval at the moved fixture corpus#81
zl190 wants to merge 1 commit into
microsoft:devfrom
zl190:fix/eval-fixtures-path

Conversation

@zl190

@zl190 zl190 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • tools/run_full_eval.py exits with ERROR: manifest.json not found and evaluates nothing: FIXTURES still points at packages/flint-py/tests/fixtures/, but the corpus moved to shared/test-data/ in 1d1ee39.
  • Repointing FIXTURES on its own would have dragged the report along with it, since line 345 derived that path as FIXTURES.parent — hence the separate REPORT_DIR.
  • Repointing the corpus and giving the report its own constant brings the tool back; it writes results.json and FULL_GALLERY_REPORT.md to the two locations where the committed copies already live.

Context

Found while investigating #80. The eval tool is the instrument that measures flint-py against the JS reference, so having it run again is the prerequisite for acting on anything in that issue.

Changes

packages/flint-py/tools/run_full_eval.py (+8 / −4):

  • FIXTURESshared/test-data/, with a comment naming the commit that moved it.
  • New REPORT_DIR = ROOT / "tests" so FULL_GALLERY_REPORT.md keeps landing next to its committed copy rather than following the corpus.
  • Module docstring updated to name both output locations.

Review focus

  1. Are those the two output locations you want? results.json goes to shared/test-data/results.json (which is tracked there already) and the report to packages/flint-py/tests/FULL_GALLERY_REPORT.md (likewise). I inferred the intent from where the committed copies sit — say the word if either belongs elsewhere.
  2. Should the regenerated outputs be part of this PR? Running the tool rewrites both files. I left them out so the diff stays at the fix. See the note below on what they now say.

Test plan

  • uv run python tools/run_full_eval.py from packages/flint-py — completes and prints: PASS: 476, MISMATCH: 180, PY_ERROR: 0, JS_ERROR: 1, FIXTURE_MISSING: 0.
  • Confirmed the two outputs land on the committed copies (git status shows exactly packages/flint-py/tests/FULL_GALLERY_REPORT.md and shared/test-data/results.json as modified, nothing untracked).
  • Cross-checked the tool's MISMATCH: 180 against uv run pytest -q, which reports 180 failures. Two independent instruments, same number.
  • Reverted both regenerated artifacts, leaving the diff at the one file.

Notes for reviewers

  • What the refreshed report says. Regenerating it turns the summary from 658 / 658 (100.0%) into 476 / 656 (72.6%). That drop is what Golden fixtures in shared/test-data no longer match the JS reference (705/705) #80 is about; it is not caused by this change, which touches only where the tool reads and writes.
  • Small pre-existing inconsistency, not fixed here. The report generator prints "There are N specs that differ … i.e. the Python port matches the JS reference on every ported chart type for every gallery test case" as one unconditional sentence, so with N > 0 it contradicts itself. Left alone to keep this diff to the paths; happy to send a follow-up.

`tools/run_full_eval.py` exits with "manifest.json not found" and
evaluates nothing. `FIXTURES` still resolves to
`packages/flint-py/tests/fixtures/`, but the corpus moved to
`shared/test-data/` in 1d1ee39 and this constant did not follow.

The report path is derived from `FIXTURES.parent`, so repointing the
corpus alone would have carried the report out of `tests/` along with
it. Giving the report its own `REPORT_DIR` keeps both outputs on the
locations where the committed copies already live: `results.json` next
to the corpus, `FULL_GALLERY_REPORT.md` under `tests/`.

With this applied the tool completes and reports PASS 476 / MISMATCH 180
/ PY_ERROR 0 / JS_ERROR 1 / FIXTURE_MISSING 0. The mismatch count agrees
with `pytest -q`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@Chenglong-MS

Copy link
Copy Markdown
Contributor

this is great, the python library is quite outdated, I think it's good that we should use the new test cases to keep them in sync.

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.

2 participants