Skip to content

Fix: unblock column_config E2E tests in CI - #5

Closed
sfc-gh-mbarnes wants to merge 1 commit into
mainfrom
fix/column-config-e2e-ci-caching
Closed

Fix: unblock column_config E2E tests in CI#5
sfc-gh-mbarnes wants to merge 1 commit into
mainfrom
fix/column-config-e2e-ci-caching

Conversation

@sfc-gh-mbarnes

Copy link
Copy Markdown
Contributor

Summary

The Tier 1 column_config feature commit (420fc1e) caused the Playwright suite to fail in this CI run. Root-cause analysis identified two distinct bugs, both fixed here.

1. Stale wheel in cached venv (CI workflow)

.github/workflows/playwright.yml caches venv with a key that only rolls daily. Same-day reruns restore the venv including a previously-installed streamlit-pivot==0.3.0 wheel. pip install -r test-requirements.txt then prints:

streamlit-pivot is already installed with the same version as the provided wheel. Use --force-reinstall to force an installation of the wheel.

…and silently skips the freshly-built wheel, leaving the test run to exercise pre-Tier-1 code. Reproduced locally byte-for-byte by installing a wheel built from 1e2edfc (pre-Tier-1) into a fresh venv, then running pip install against a same-versioned fresh wheel.

Fix: after pip install -r test-requirements.txt, force-reinstall the local wheel:

pip install --force-reinstall --no-deps dist/streamlit_pivot-*.whl

--no-deps keeps the dep cache useful; the wheel is ~900 KB so the extra step adds ~1s. A comment documents why.

2. Unreachable measure-header assertions (test app)

test_column_config_label_measure and test_column_config_help_tooltip_measure assert on <th data-testid="pivot-header-cell">, which TableRenderer.tsx only emits on the config.columns.length === 0 branch (renderers/TableRenderer.tsx line ~1520). The two apps used columns=["Year"] with a single value, so that cell never rendered — the assertions failed even in an editable install against HEAD, independent of the caching bug.

Fix: drop columns=["Year"] from test_pivot_cc_label and test_pivot_cc_help so the single-value measure header renders.

Verification

Simulated the CI scenario locally end-to-end:

  1. Built a "stale" wheel from 1e2edfc (pre-Tier-1), installed into a fresh venv (simulates cache restore).
  2. Installed a freshly-built wheel via pip install -r <reqs> → pip printed "already installed with the same version" and skipped. Venv retained 0 occurrences of field_labels / field_renderers in Python and JS (matches CI symptom).
  3. Applied the workflow fix (--force-reinstall --no-deps) → venv now has 13 / 5 occurrences of field_labels matching HEAD.
  4. Ran the full pivot_table_column_config_test.py suite against the force-reinstalled venv: 7/7 pass (up from 0). Same result in the local editable-install venv.

Test plan

  • Full pivot_table_column_config_test.py passes locally (chromium): 7/7.
  • Full pivot_table_column_config_test.py passes against the simulated-CI venv with both fixes applied: 7/7.
  • Pre-commit hooks pass (ruff, ruff-format, license headers, formatting).
  • Playwright workflow passes on this PR in CI (webkit, chromium, firefox).

Made with Cursor

The Tier 1 column_config changes (420fc1e) caused widespread Playwright
failures. Two distinct bugs were at play:

1. Stale wheel in cached venv. `.github/workflows/playwright.yml` caches
   `venv` with a key that only rolls daily. Subsequent same-day runs
   restore the venv including a previous `streamlit-pivot==0.3.0` wheel.
   `pip install -r test-requirements.txt` then sees "Requirement already
   satisfied" and silently skips the freshly-built wheel in `dist/`,
   leaving the test run to exercise pre-Tier-1 code. Force-reinstall the
   local wheel with `--force-reinstall --no-deps` so the venv always
   reflects HEAD, regardless of cache hits.

2. Unreachable measure-header assertions. The two measure-header tests
   (`test_column_config_label_measure`, `test_column_config_help_tooltip_measure`)
   target `<th data-testid="pivot-header-cell">`, which the renderer only
   emits on the `config.columns.length === 0` branch. The app configs used
   `columns=["Year"]` with a single value, so that cell never rendered and
   the assertions had nothing to match. Drop the col dim from the two
   affected apps so the single-value measure header renders.

Verified end-to-end by simulating the CI cache scenario locally
(stale 0.3.0 wheel preinstalled, then the fixed workflow steps): all 7
column_config tests pass (previously 0).

Made-with: Cursor
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