-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
Moved sqllab tests from core_tests to sqllab_tests #1502
Conversation
@@ -176,3 +176,14 @@ def revoke_public_access(self, table_name): | |||
if (perm.permission.name == 'datasource_access' and | |||
perm.view_menu and table_name in perm.view_menu.name): | |||
appbuilder.sm.del_permission_role(public_role, perm) | |||
|
|||
def run_sql(self, sql, user_name, client_id): |
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.
make user_name optional and if none run as admin
self.run_sql("SELECT * FROM ab_permission", 'gamma', client_id='client_id_2') | ||
|
||
def tearDown(self): | ||
pass |
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.
db.session.query(models.Query).delete()
do the same in core tests
|
||
def test_sql_json(self): | ||
data = self.run_sql('SELECT * FROM ab_user', 'admin', "1") | ||
assert len(data['data']) > 0 |
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.
while you are here, could you please refactor asserts into assertEquals, assertLess, etc ?
minor comments, looks good overall ! |
ae92b37
to
6f9477d
Compare
ed22cdb
to
b34cd22
Compare
Why:
Done:
needs-review @mistercrunch @bkyryliuk