-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
test: DataTableControl #13668
test: DataTableControl #13668
Conversation
Codecov Report
@@ Coverage Diff @@
## master #13668 +/- ##
==========================================
- Coverage 76.46% 73.09% -3.38%
==========================================
Files 921 611 -310
Lines 46769 21755 -25014
Branches 5644 5665 +21
==========================================
- Hits 35761 15901 -19860
+ Misses 10848 5718 -5130
+ Partials 160 136 -24
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some small nits. Code LGTM.
superset-frontend/src/explore/components/DataTableControl/RowCount.test.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/src/explore/components/DataTableControl/RowCount.test.tsx
Outdated
Show resolved
Hide resolved
…ount.test.tsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
…ount.test.tsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
test('Render a button', () => { | ||
render(<CopyButton>btn</CopyButton>); | ||
expect(screen.getByRole('button')).toBeInTheDocument(); | ||
expect(screen.getByRole('button')).toHaveClass('superset-button'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just out of curiosity, why do you think checking the class here would benefit the tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't really add much value. It’s just because a test that just checks that it’s a button doesn’t add much... At least with the class I check if "added the correct button"
@rusackas please take a look here! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
* master: (26 commits) chore: bump to new superset-ui version (#13932) fix: do not run containers as root by default in Helm chart (#13917) feat(explore): adhoc column formatting for Table chart (#13758) fix(sqla-query): order by aggregations in Presto and Hive (#13739) feat(alert/report): add ALERTS_ATTACH_REPORTS feature flags + feature (#13894) test: Fixes PropertiesModal_spec (#13548) fix: Pin Prophet dependency after breaking changes (#13852) test: Adds tests to dnd controls (#13650) test: Adds tests to the AnnotationLayer component (#13748) test: Refactor and enhance tests for the Explore DatasourcePanel Component (#13799) Add tests (#13778) test: DisplayQueryButton (#13750) Fixing condition around left margin for dashboard layout. Fixes #13863 (#13905) Revert "fix: select table overlay (#13694)" (#13901) test: Adds tests to the OptionControls component (#13729) test: DatasourceControl (#13605) tests for function handleScroll (#13896) test: Adds tests to the CustomFrame component (#13675) test: Adds tests to the AdvancedFrame component (#13664) test: DataTableControl (#13668) ...
* Tests for DataTableControl * Update superset-frontend/src/explore/components/DataTableControl/RowCount.test.tsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> * Update superset-frontend/src/explore/components/DataTableControl/RowCount.test.tsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
* Tests for DataTableControl * Update superset-frontend/src/explore/components/DataTableControl/RowCount.test.tsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> * Update superset-frontend/src/explore/components/DataTableControl/RowCount.test.tsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
SUMMARY
Tests for DataTableControl file
TEST PLAN
No behavior changes. All tests must pass.