Skip to content

test(stand): make the download journeys prove what the file holds - #2597

Merged
ktursunov merged 1 commit into
mainfrom
codex/stand-download-journeys
Aug 17, 2026
Merged

test(stand): make the download journeys prove what the file holds#2597
ktursunov merged 1 commit into
mainfrom
codex/stand-download-journeys

Conversation

@ktursunov

@ktursunov ktursunov commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

tests/stand/ui/ holds the browser tests that click through a deployed Insight
like a user would. Three of them open a dialog and export a file —
test_git_output_timeseries, test_collaboration_card_evidence,
test_metric_evidence_drilldown — and each checked only that the filename ended
in .csv or .xlsx and that the file was bigger than zero bytes. A file holding
the wrong period, a header and no rows, or zeros where the screen showed blanks
passes that too.

What they assert now

Before Now
filename ends in .csv / .xlsx same, and the file is opened and read back
file is larger than 0 bytes CSV and XLSX must hold the same table as each other
the exported record count equals the row count the grid itself declares (aria-rowcount)
every row visible on screen equals the row exported for it

No expected value is written into a test, so re-seeding the stand cannot break
them. Each comparison is between two things the stand produced independently:
what the SPA rendered, and what the export path serialized.

How

One new helper, tests/stand/ui/downloads.py. It saves the download, reads
.csv with the standard library and .xlsx with openpyxl (new dependency,
locked), and puts every cell — CSV text, XLSX number, rendered DOM text — through
one normalizer. 29, 29.0 and a rendered +2,705 compare equal, while an empty
cell stays distinct from a zero. That last distinction is the point: a serializer
that writes 0 for a missing value passes anything that merely counts rows.

Verification

All 17 tests in tests/stand/ui pass against a seeded compose stand. To confirm
the new assertions can fail, the reader was broken three ways on purpose — a
missing cell read as 0, the last record dropped, every number shifted — and the
three tests failed each time.

One call for the reviewer

The XLSX export of the Git output timeseries carries the Total and Grand total
rows the table shows; the CSV stops at the last bucket. The test compares the
bucket rows only, so it takes no position on which is right — but one of them
probably is.

Three browser journeys downloaded a CSV or XLSX and asserted that the filename
ended in the right suffix and the file was longer than zero bytes. That passes
on a file holding the wrong scope, the wrong period, a header and no rows, or
zeros where the grid showed blanks — every failure the export can actually
have.

`downloads.py` saves the download and parses it: `csv` for one format,
`openpyxl` for the other, with one normalizer over CSV strings, XLSX numbers
and rendered DOM text so `29`, `29.0` and a formatted `+2,705` compare equal
while an empty cell stays distinct from a zero. Nothing is written down as an
expected value — each journey now reconciles the two formats against each
other, the record count against the grid's own `aria-rowcount`, and the
rendered rows against the exported ones, so the assertions hold for whatever
the stand was seeded with.

Verified against a seeded compose stand: the three journeys pass, the whole
`tests/stand/ui` suite passes, and each new assertion was checked against a
deliberately broken reader — a missing cell serialized as zero, an export
losing its last record, and files carrying values the grid never rendered all
fail the journeys that should catch them.

Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown

Important

Review available on request

  • 🔍 Trigger review

Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment @coderabbitai review to review the latest changes. For a full review, comment @coderabbitai full review.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f419d8fd-57a2-4bb6-9b03-786b5a3f5b54


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@ktursunov
ktursunov marked this pull request as ready for review August 17, 2026 09:24
@ktursunov
ktursunov requested a review from a team as a code owner August 17, 2026 09:24
@ktursunov
ktursunov added this pull request to the merge queue Aug 17, 2026
Merged via the queue into main with commit 72da6ff Aug 17, 2026
27 checks passed
@ktursunov
ktursunov deleted the codex/stand-download-journeys branch August 17, 2026 10:48
@ktursunov

Copy link
Copy Markdown
Contributor Author

Correcting my own note here: the CSV/XLSX footer difference is by design, not a defect.

24fe8baf fix(metrics): omit CSV footer rows removed the footers from the CSV deliberately, and metric-timeseries-export.test.ts locks both halves — "exports grouped CSV data without footer rows" asserts the CSV contains neither Total nor Grand total, while the workbook test asserts A6/A7 hold them. The reasoning holds up: the workbook's grand total is a formatted composite string in a merged cell, which would corrupt the CSV's machine-readable shape.

So there is nothing to decide and nothing to file. What was worth fixing is this journey, which compared bucket rows only and therefore asserted neither half of that contract — #2608 asserts the split in both directions.

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