Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: tests errors and warnings - iteration 1 (#12212) #12213

Merged

Conversation

michael-s-molina
Copy link
Member

@michael-s-molina michael-s-molina commented Dec 28, 2020

SUMMARY

Remove tests errors and warnings to improve results readability.

The following errors and warnings have been removed:

Warning: Invalid DOM property `autocomplete`. Did you mean `autoComplete`?

spec/javascripts/explore/components/SaveModal_spec.jsx
spec/javascripts/datasource/DatasourceModal_spec.jsx
spec/javascripts/sqllab/TabbedSqlEditors_spec.jsx
spec/javascripts/explore/components/ColorScheme_spec.jsx
spec/javascripts/explore/components/FilterBox_spec.jsx
spec/javascripts/views/CRUD/chart/ChartList_spec.jsx
src/views/CRUD/data/query/QueryList.test.tsx
spec/javascripts/views/CRUD/data/database/DatabaseList_spec.jsx
spec/javascripts/views/CRUD/alert/AlertReportModal_spec.jsx
spec/javascripts/components/TableSelector_spec.jsx
spec/javascripts/components/ListView/ListView_spec.jsx
spec/javascripts/dashboard/components/PropertiesModal_spec.jsx
spec/javascripts/views/CRUD/data/savedquery/SavedQueryList_spec.jsx
spec/javascripts/views/CRUD/csstemplates/CssTemplatesList_spec.jsx
spec/javascripts/views/CRUD/alert/AlertList_spec.jsx
spec/javascripts/views/CRUD/data/dataset/DatasetList_spec.jsx
spec/javascripts/views/CRUD/annotationlayers/AnnotationLayersList_spec.jsx
spec/javascripts/components/SupersetResourceSelect_spec.tsx
Warning: React does not recognize the `activeKey` prop on a DOM element. 
If you intentionally want it to appear in the DOM as a custom attribute, 
spell it as lowercase `activekey` instead. If you accidentally passed it from a parent component, 
remove it from the DOM element.

spec/javascripts/views/CRUD/dashboard/DashboardList_spec.jsx
spec/javascripts/views/CRUD/chart/ChartList_spec.jsx
src/views/CRUD/data/query/QueryList.test.tsx
spec/javascripts/views/CRUD/data/database/DatabaseList_spec.jsx
spec/javascripts/views/CRUD/welcome/SavedQueries_spec.tsx
spec/javascripts/views/CRUD/data/savedquery/SavedQueryList_spec.jsx
spec/javascripts/views/CRUD/csstemplates/CssTemplatesList_spec.jsx
spec/javascripts/views/CRUD/alert/AlertList_spec.jsx
spec/javascripts/views/CRUD/data/dataset/DatasetList_spec.jsx
spec/javascripts/views/CRUD/annotationlayers/AnnotationLayersList_spec.jsx
spec/javascripts/views/CRUD/annotation/AnnotationList_spec.jsx
spec/javascripts/views/CRUD/welcome/ActivityTable_spec.tsx
spec/javascripts/views/CRUD/welcome/DashboardTable_spec.tsx
spec/javascripts/views/CRUD/welcome/Welcome_spec.tsx
spec/javascripts/views/CRUD/welcome/ChartTable_spec.tsx
Warning: React does not recognize the `activeHref` prop on a DOM element. 
If you intentionally want it to appear in the DOM as a custom attribute, 
spell it as lowercase `activehref` instead. If you accidentally passed it from a parent component, 
remove it from the DOM element.

spec/javascripts/views/CRUD/dashboard/DashboardList_spec.jsx
spec/javascripts/views/CRUD/chart/ChartList_spec.jsx
src/views/CRUD/data/query/QueryList.test.tsx
spec/javascripts/views/CRUD/data/database/DatabaseList_spec.jsx
spec/javascripts/views/CRUD/welcome/SavedQueries_spec.tsx
spec/javascripts/views/CRUD/data/savedquery/SavedQueryList_spec.jsx
spec/javascripts/views/CRUD/csstemplates/CssTemplatesList_spec.jsx
spec/javascripts/views/CRUD/alert/AlertList_spec.jsx
spec/javascripts/views/CRUD/data/dataset/DatasetList_spec.jsx
spec/javascripts/views/CRUD/annotationlayers/AnnotationLayersList_spec.jsx
spec/javascripts/views/CRUD/annotation/AnnotationList_spec.jsx
spec/javascripts/views/CRUD/welcome/ActivityTable_spec.tsx
spec/javascripts/views/CRUD/welcome/DashboardTable_spec.tsx
spec/javascripts/views/CRUD/welcome/Welcome_spec.tsx
spec/javascripts/views/CRUD/welcome/ChartTable_spec.tsx
Warning: Each child in a list should have a unique "key" prop.

spec/javascripts/explore/components/AdhocFilterEditPopoverSimpleTabContent_spec.jsx
Warning: validateDOMNesting(...): <div> cannot appear as a descendant of <p>.

spec/javascripts/views/CRUD/welcome/DashboardTable_spec.tsx
spec/javascripts/views/CRUD/welcome/Welcome_spec.tsx
spec/javascripts/views/CRUD/welcome/ChartTable_spec.tsx
spec/javascripts/views/CRUD/welcome/EmptyState_spec.tsx
Warning: Failed prop type: You have provided a `open` prop to `Dropdown` 
without an `onToggle` handler prop. This will render a read-only field. 
If the field should be mutable use `defaultOpen`. Otherwise, set `onToggle`.

src/components/Menu/Menu.tsx
src/components/Menu/MenuObject.tsx
src/components/Menu/NewMenu.tsx

#12212

@rusackas @junlincc @villebro

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

@michael-s-molina michael-s-molina changed the title Fix tests errors and warnings - iteration 1 (#12212) fix: tests errors and warnings - iteration 1 (#12212) Dec 29, 2020
@junlincc
Copy link
Member

junlincc commented Jan 1, 2021

thanks for the PRs @michael-s-molina! is there a test plan for them?

@junlincc junlincc requested review from rusackas and ktmud and removed request for rusackas January 1, 2021 22:24
@michael-s-molina
Copy link
Member Author

thanks for the PRs @michael-s-molina! is there a test plan for them?

We just need to run the tests and verify that the errors and warnings are decreasing. Most of the changes are in spec files and the ones that change a component are covered by a test.

@villebro villebro closed this Jan 4, 2021
@villebro villebro reopened this Jan 4, 2021
@codecov-io
Copy link

codecov-io commented Jan 4, 2021

Codecov Report

Merging #12213 (6cf6acc) into master (f3ab1f4) will decrease coverage by 2.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #12213      +/-   ##
==========================================
- Coverage   66.20%   64.19%   -2.01%     
==========================================
  Files         996      485     -511     
  Lines       49174    29830   -19344     
  Branches     4993        0    -4993     
==========================================
- Hits        32554    19149   -13405     
+ Misses      16476    10681    -5795     
+ Partials      144        0     -144     
Flag Coverage Δ
cypress ?
javascript ?
python 64.19% <ø> (-0.03%) ⬇️

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

Impacted Files Coverage Δ
superset/cli.py 33.58% <0.00%> (-0.92%) ⬇️
superset/db_engine_specs/presto.py 82.68% <0.00%> (-0.65%) ⬇️
superset/async_events/api.py 100.00% <0.00%> (ø)
superset-frontend/src/components/Pagination.tsx
...ntend/src/components/dataViewCommon/Pagination.tsx
superset-frontend/src/utils/parseCookie.ts
...d/src/dashboard/util/getLeafComponentIdFromPath.js
...frontend/src/SqlLab/components/QueryStateLabel.jsx
superset-frontend/src/views/CRUD/alert/types.ts
...ontend/src/dashboard/components/dnd/handleHover.js
... and 497 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 f3ab1f4...6cf6acc. Read the comment docs.

@ktmud
Copy link
Member

ktmud commented Jan 4, 2021

Hi, @michael-s-molina , thanks for the contribution! This looks great. I know this is probably too much to ask, but could you comment on each change on which errors/warnings they are supposed to solve if you still remember? PR review comments are fine.

@michael-s-molina
Copy link
Member Author

michael-s-molina commented Jan 5, 2021

@ktmud I updated the PR description to include fixed errors and warnings. I'll do the same with the other PRs.

@ktmud
Copy link
Member

ktmud commented Jan 5, 2021

Thanks so much!

Copy link
Member

@ktmud ktmud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a couple of small nits.

@michael-s-molina michael-s-molina force-pushed the fix-tests-errors-warnings-1 branch from e4da213 to 6cf6acc Compare January 5, 2021 16:59
@michael-s-molina
Copy link
Member Author

@ktmud Accepted suggested changes. I did a force push to preserve commit history.

@ktmud ktmud closed this Jan 5, 2021
@ktmud ktmud reopened this Jan 5, 2021
@rusackas rusackas merged commit 1b908ab into apache:master Jan 6, 2021
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.

6 participants