-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
refactor(QueryObject): decouple from superset #17479
refactor(QueryObject): decouple from superset #17479
Conversation
c73f121
to
1749193
Compare
1749193
to
c7cacd3
Compare
9ef9dd7
to
9dc39fa
Compare
Codecov Report
@@ Coverage Diff @@
## master #17479 +/- ##
=======================================
Coverage 76.87% 76.88%
=======================================
Files 1042 1043 +1
Lines 56345 56369 +24
Branches 7793 7793
=======================================
+ Hits 43316 43337 +21
- Misses 12773 12776 +3
Partials 256 256
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.
decoupling of query object from superset shows value already by having simple unit-tests for the query_object factory, Nice!
* refactor: queryObject - decouple from superset * refactor: queryObject - decouple from superset
Background
When we have worked on #16991 we wanted to test the new functionalities in concrete and accurate unittest.
All chartData flows and its components are too couple to superset so it is impossible to create unittests.
The flows are not testable and so many components do not meet the very important principle SRP and the code became so dirty
So I've started to refactor it (#17344 ) but many changes were added and it was hard to review so I decided to split those changes into small PRs so will be easier to follow
This is the ninth PR in a sequence of PRs to meet these
The next PR is #17495
PR description
Test plans
There is no logic added so new tests are not required
Previous PRs