Skip to content

test: fix table chart sort order E2E tests#12936

Merged
villebro merged 2 commits intoapache:masterfrom
ktmud:table-e2e-test
Feb 4, 2021
Merged

test: fix table chart sort order E2E tests#12936
villebro merged 2 commits intoapache:masterfrom
ktmud:table-e2e-test

Conversation

@ktmud
Copy link
Copy Markdown
Member

@ktmud ktmud commented Feb 4, 2021

SUMMARY

Follow up of #12930 . Fix the table chart E2E test currently failing in master. The test expects table chart to be sorted by first metric in descending order when a "Sort by" metric is not set, regardless of if "Sort descending" is selected. This is the original logic pre table chart API migration (#10270).

Needs to wait for apache-superset/superset-ui#935

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

After

Correctly sorted by first metric in descending order:

Snip20210203_1

Before

Incorrectly sort by first metric in ascending order.

TEST PLAN

CI and

  1. Go to table chart
  2. Select a metric, don't select sort by metric, don't check "sort descending"
  3. Your chart should sort by first metric in descending order

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • Introduces new feature or API
  • Removes existing feature or API

@junlincc junlincc added the viz:charts:table Related to the Table chart label Feb 4, 2021
@codecov-io
Copy link
Copy Markdown

codecov-io commented Feb 4, 2021

Codecov Report

Merging #12936 (2f14e88) into master (94b6b29) will decrease coverage by 1.16%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #12936      +/-   ##
==========================================
- Coverage   68.38%   67.22%   -1.17%     
==========================================
  Files        1025      489     -536     
  Lines       48764    28669   -20095     
  Branches     5189        0    -5189     
==========================================
- Hits        33348    19273   -14075     
+ Misses      15273     9396    -5877     
+ Partials      143        0     -143     
Flag Coverage Δ
cypress ?
javascript ?
python 67.22% <ø> (-0.41%) ⬇️

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

Impacted Files Coverage Δ
superset/db_engines/hive.py 0.00% <0.00%> (-85.72%) ⬇️
superset/db_engine_specs/hive.py 73.84% <0.00%> (-16.93%) ⬇️
superset/db_engine_specs/presto.py 81.38% <0.00%> (-6.93%) ⬇️
superset/db_engine_specs/sqlite.py 90.62% <0.00%> (-6.25%) ⬇️
superset/utils/celery.py 86.20% <0.00%> (-3.45%) ⬇️
superset/views/database/mixins.py 80.70% <0.00%> (-1.76%) ⬇️
superset/result_set.py 96.69% <0.00%> (-1.66%) ⬇️
superset/db_engine_specs/base.py 85.01% <0.00%> (-0.99%) ⬇️
superset/models/core.py 88.04% <0.00%> (-0.82%) ⬇️
superset/connectors/sqla/models.py 90.28% <0.00%> (-0.27%) ⬇️
... and 536 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94b6b29...2f14e88. Read the comment docs.

@pull-request-size pull-request-size bot added size/M and removed size/S labels Feb 4, 2021
@amitmiran137
Copy link
Copy Markdown
Member

this is great . those tests are failing and flaky on master

@ktmud
Copy link
Copy Markdown
Member Author

ktmud commented Feb 4, 2021

@villebro @amitmiran137 feel free to merge this when CI is green.

@villebro villebro merged commit 9982fde into apache:master Feb 4, 2021
villebro pushed a commit to preset-io/superset that referenced this pull request Feb 4, 2021
* test: fix table chart sort order E2E tests

* Upgrade npm packages
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.2.0 First shipped in 1.2.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/M viz:charts:table Related to the Table chart 🚢 1.2.0 First shipped in 1.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants