Skip to content

chore(cypress): remove dead _skip spec files and skipped inline tests#40384

Merged
rusackas merged 1 commit into
masterfrom
chore/cypress-remove-skipped-tests
May 23, 2026
Merged

chore(cypress): remove dead _skip spec files and skipped inline tests#40384
rusackas merged 1 commit into
masterfrom
chore/cypress-remove-skipped-tests

Conversation

@rusackas
Copy link
Copy Markdown
Member

SUMMARY

  • Deletes 27 _skip.* spec files that are permanently excluded from CI via excludeSpecPattern: ['**/_skip.*'] in cypress.config.ts. These files never run and exist only as dead code.
  • Strips 22 it.skip() tests from editmode.test.ts — all belong to the Color consistency describe block, permanently skipped because ECharts renders to canvas and Cypress cannot inspect canvas elements. The 1 active it('should add charts') test is preserved along with the minimal helpers it needs.
  • No active tests are removed. No CI behavior changes.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A — no UI changes.

TESTING INSTRUCTIONS

  • Confirm cypress.config.ts still lists excludeSpecPattern: ['**/_skip.*'] (no longer needed for the deleted files, but no change required)
  • Run the Cypress suite and verify editmode.test.ts still executes the should add charts test
  • Verify no other spec files reference the deleted _skip.* files

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

🤖 Generated with Claude Code

The 27 _skip.* spec files are already excluded from CI by
`excludeSpecPattern` in cypress.config.ts — they never run and only
exist as dead code. Delete them.

Also strip the 22 it.skip() tests from editmode.test.ts (all in the
"Color consistency" describe block, permanently skipped because ECharts
renders to canvas which Cypress can't inspect). The 1 active test is
preserved.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented May 22, 2026

Code Review Agent Run #2cfc1c

Actionable Suggestions - 0
Additional Suggestions - 10
  • superset-frontend/cypress-base/cypress/e2e/dashboard/editmode.test.ts - 3
    • Missing Playwright test coverage · Line 476-825
      Removing 9 skipped Cypress test cases (~350 lines) covering color scheme and label color behavior in nested tabs. Per AGENTS.md, Cypress is deprecated and Playwright migration is underway. Verify that unit tests cover the color scheme logic and that Playwright E2E tests will be added to maintain coverage of this dashboard functionality.
    • Misleading comment about test coverage · Line 1110-1112
      Comment at line 1110-1112 states that PropertiesModal Jest tests cover the removed modal color scheme functionality, but the referenced file only tests basic metadata save/restore with `supersetColors` — it does not cover any of the 6 deleted cross-tab/nested-tab color scheme test cases. The misleading comment could cause future maintainers to believe coverage exists when it does not.
    • Unverifiable comment citing missing test coverage · Line 1114-1118
      Comments at lines 1114-1118 claim that 'edit mode functionality' and 'drag/drop workflows' are covered by Jest integration tests, but no specific test files or paths are cited. Without concrete references, these comments provide no actionable guidance to future maintainers and may mask gaps in coverage.
  • superset-frontend/cypress-base/cypress/e2e/explore/visualizations/_skip.compare.test.js - 1
    • Test coverage gap · Line 1-100
      Deleting Cypress E2E test for Compare chart removes existing (albeit skipped) test coverage. Per AGENTS.md testing strategy, Cypress is deprecated in favor of Playwright. No replacement tests identified. This creates a test coverage gap for the Compare chart visualization (CompareChartPlugin at `plugins/legacy-preset-chart-nvd3/src/Compare/`). Consider adding Playwright or unit tests before removing coverage entirely.
  • superset-frontend/cypress-base/cypress/e2e/explore/visualizations/_skip.table.test.ts - 1
    • E2E Integration Test Coverage Gap · Line 1-352
      Deleting 352-line Cypress test file removes coverage for 13 E2E integration scenarios (server pagination with error validation, time formatting by granularity, frontend/backend sorting, query mode switching, groupby with metrics/limits) without creating replacement tests. Unit tests in `plugin-chart-table/test/TableChart.test.tsx` cover component behavior, not E2E integration flows. Per project guidelines, Cypress is deprecated and Playwright is the target, but no Playwright tests were added for these scenarios. This creates a regression risk for complex server/client interaction patterns in the Table visualization.
  • superset-frontend/cypress-base/cypress/e2e/explore/visualizations/_skip.big_number.test.js - 1
    • Missing E2E test coverage · Line 1-80
      Deleting the only E2E test for the 'big_number' visualization (with Trendline). Per AGENTS.md, Cypress is deprecated and Playwright is the standard. No equivalent Playwright test found for this chart type. Ensure E2E coverage gap is intentional and documented, or provide a replacement test.
  • superset-frontend/cypress-base/cypress/e2e/explore/visualizations/_skip.bubble.test.js - 1
    • Filter behavior verification missing · Line 59-88
      The deleted test 'should work with filter' validated that filtering by region='South Asia' produces 8 circle elements in `.nv-point-clips` and that all circles have matching `fill` attributes. This behavioral verification is not covered by unit tests which mock data flow.
  • superset-frontend/cypress-base/cypress/e2e/explore/_skip.annotations.test.ts - 1
    • E2E test coverage gap · Line 1-48
      The deleted Cypress E2E test covered end-to-end flow: loading a chart ('Num Births Trend'), creating a formula annotation (y=1400000), running the query, and verifying the annotation renders in the chart UI. Unit tests in `AnnotationLayer.test.tsx` verify component-level behavior (form inputs, button states, callbacks) but do NOT cover the integration of formula annotations rendering within actual chart output. Consider migrating this to Playwright or documenting coverage gap.
  • superset-frontend/cypress-base/cypress/e2e/dashboard/_skip.horizontalFilterBar.test.ts - 1
    • Dead code from orphaned utilities · Line 1-292
      Deleting this deprecated Cypress test aligns with AGENTS.md migration strategy (Playwright over Cypress). However, the utility functions `testItems`, `interceptFilterState`, `addCountryNameFilter`, `applyNativeFilterValueWithIndex`, and `inputNativeFilterDefaultValue` in `utils.ts` are now dead code - none are used by remaining Cypress tests or Playwright tests. Consider removing them to prevent maintenance burden.
  • superset-frontend/cypress-base/cypress/e2e/explore/visualizations/_skip.box_plot.test.js - 1
    • E2E coverage gap · Line 1-65
      This deletion removes a permanently skipped Cypress E2E test that was already excluded from CI via `excludeSpecPattern`. No replacement test coverage exists for Box Plot E2E behavior (color scheme search interaction). Unit tests for `boxplotOperator` exist in `superset-ui-chart-controls`. If E2E coverage is needed, consider adding Playwright tests per the migration strategy in AGENTS.md.
Filtered by Review Rules

Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.

  • superset-frontend/cypress-base/cypress/e2e/dashboard/editmode.test.ts - 2
    • Missing test coverage for tab color scheme · Line 859-958
    • Missing test coverage for color scheme override · Line 826-858
  • superset-frontend/cypress-base/cypress/e2e/explore/visualizations/_skip.sunburst.test.js - 1
    • Missing test replacement coverage · Line 1-97
Review Details
  • Files reviewed - 28 · Commit Range: 34b5669..34b5669
    • superset-frontend/cypress-base/cypress/e2e/dashboard/_skip.controls.test.ts
    • superset-frontend/cypress-base/cypress/e2e/dashboard/_skip.horizontalFilterBar.test.ts
    • superset-frontend/cypress-base/cypress/e2e/dashboard/_skip.key_value.test.ts
    • superset-frontend/cypress-base/cypress/e2e/dashboard/_skip.load.test.ts
    • superset-frontend/cypress-base/cypress/e2e/dashboard/_skip.nativeFilters.noInitState.test.ts
    • superset-frontend/cypress-base/cypress/e2e/dashboard/_skip.nativeFilters.test.ts
    • superset-frontend/cypress-base/cypress/e2e/dashboard/_skip.tabs.test.ts
    • superset-frontend/cypress-base/cypress/e2e/dashboard/_skip.url_params.test.ts
    • superset-frontend/cypress-base/cypress/e2e/dashboard/editmode.test.ts
    • superset-frontend/cypress-base/cypress/e2e/explore/_skip.AdhocFilters.test.ts
    • superset-frontend/cypress-base/cypress/e2e/explore/_skip.AdhocMetrics.test.ts
    • superset-frontend/cypress-base/cypress/e2e/explore/_skip.advanced_analytics.test.ts
    • superset-frontend/cypress-base/cypress/e2e/explore/_skip.annotations.test.ts
    • superset-frontend/cypress-base/cypress/e2e/explore/_skip.link.test.ts
    • superset-frontend/cypress-base/cypress/e2e/explore/visualizations/_skip.big_number.test.js
    • superset-frontend/cypress-base/cypress/e2e/explore/visualizations/_skip.big_number_total.test.js
    • superset-frontend/cypress-base/cypress/e2e/explore/visualizations/_skip.box_plot.test.js
    • superset-frontend/cypress-base/cypress/e2e/explore/visualizations/_skip.bubble.test.js
    • superset-frontend/cypress-base/cypress/e2e/explore/visualizations/_skip.compare.test.js
    • superset-frontend/cypress-base/cypress/e2e/explore/visualizations/_skip.download_chart.test.js
    • superset-frontend/cypress-base/cypress/e2e/explore/visualizations/_skip.gauge.test.js
    • superset-frontend/cypress-base/cypress/e2e/explore/visualizations/_skip.graph.test.ts
    • superset-frontend/cypress-base/cypress/e2e/explore/visualizations/_skip.pie.test.js
    • superset-frontend/cypress-base/cypress/e2e/explore/visualizations/_skip.pivot_table.test.js
    • superset-frontend/cypress-base/cypress/e2e/explore/visualizations/_skip.sunburst.test.js
    • superset-frontend/cypress-base/cypress/e2e/explore/visualizations/_skip.table.test.ts
    • superset-frontend/cypress-base/cypress/e2e/explore/visualizations/_skip.time_table.js
    • superset-frontend/cypress-base/cypress/e2e/explore/visualizations/_skip.world_map.test.js
  • Files skipped - 0
  • Tools
    • Eslint (Linter) - ✔︎ Successful
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@codecov
Copy link
Copy Markdown

codecov Bot commented May 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.21%. Comparing base (e40648d) to head (34b5669).

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #40384   +/-   ##
=======================================
  Coverage   64.21%   64.21%           
=======================================
  Files        2592     2592           
  Lines      139167   139167           
  Branches    32312    32312           
=======================================
  Hits        89361    89361           
  Misses      48273    48273           
  Partials     1533     1533           
Flag Coverage Δ
javascript 67.33% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rusackas rusackas merged commit 168b49b into master May 23, 2026
74 checks passed
@rusackas rusackas deleted the chore/cypress-remove-skipped-tests branch May 23, 2026 04:11
kasiazjc pushed a commit that referenced this pull request May 26, 2026
…#40384)

Co-authored-by: Claude <claude@anthropic.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants