test(table): remove conditionals from TableChart tests #36149
Merged
sadpandajoe merged 4 commits intomasterfrom Nov 21, 2025
Merged
test(table): remove conditionals from TableChart tests #36149sadpandajoe merged 4 commits intomasterfrom
sadpandajoe merged 4 commits intomasterfrom
Conversation
Contributor
Code Review Agent Run #1089e0Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors TableChart tests to eliminate conditional logic, following the testing best practice "tests should not contain conditionals." The original tests used if (labelledBy) conditionals which could cause assertion skipping, potentially masking issues.
Key changes:
- Split 2 tests containing conditionals into 4 unconditional tests with explicit assertions
- Enhanced ARIA accessibility validation to ensure ALL cells have proper
aria-labelledbyattributes - Added explicit checks to catch empty tables and unlabeled cells
superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx
Outdated
Show resolved
Hide resolved
…IA validation Remove conditional logic from table tests following the principle that tests should not contain conditionals. Split 2 tests with if statements into 4 unconditional tests with explicit assertions. Changes: - Remove if(labelledBy) guards from ARIA validation tests - Split "header IDs" tests to separate ARIA validation into new tests - Add complete coverage check: all tbody td cells must have aria-labelledby - Add empty table guard to catch regressions (prevents silent 0===0 pass) - Replace non-null assertions with explicit truthy checks for clearer errors - Scope queries to tbody td (excludes footer cells which legitimately lack labels) Benefits: - No hidden code paths - all assertions always run - Better error messages when attributes missing - 100% data cell coverage for accessibility - Catches empty table regressions - Type-safe without suppressions All 47 tests passing with improved coverage and clarity. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
sadpandajoe
force-pushed
the
fix-table-test-conditional-statements
branch
from
7e84731 to
f86bbe8
Compare
superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx
Outdated
Show resolved
Hide resolved
betodealmeida
approved these changes
Nov 21, 2025
LuisSanchez
pushed a commit
to LuisSanchez/superset
that referenced
this pull request
Nov 25, 2025
Co-authored-by: Claude <noreply@anthropic.com>
kshi020302
pushed a commit
to jl141/superset
that referenced
this pull request
Nov 30, 2025
Co-authored-by: Claude <noreply@anthropic.com>
Facyla
pushed a commit
to Facyla/superset-contrib
that referenced
this pull request
Dec 16, 2025
Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SUMMARY
In #35968 I introduced tests with conditionals in it which could prevent parts of the validation to not run. This PR corrects this. Remove conditional logic from table tests following the testing best practice: "tests should not contain conditionals." Split 2 tests with
ifstatements into 4 unconditional tests with explicit assertions, and significantly improve ARIA accessibility validation.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION