You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/10/01 18:43:36 UTC
[GitHub] [incubator-superset] kkucharc opened a new pull request #11131: [WIP] fix: removed unicode_test example from unit tests
kkucharc opened a new pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TEST PLAN
<!--- What steps should be taken to verify the changes -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] 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
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#issuecomment-702807042
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=h1) Report
> Merging [#11131](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/d056e3dfd3a2dc4844f836979eb474abbbce4632?el=desc) will **decrease** coverage by `5.06%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #11131 +/- ##
==========================================
- Coverage 65.79% 60.73% -5.07%
==========================================
Files 816 391 -425
Lines 38422 24483 -13939
Branches 3621 0 -3621
==========================================
- Hits 25280 14869 -10411
+ Misses 13034 9614 -3420
+ Partials 108 0 -108
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `?` | |
| #python | `60.73% <ø> (-0.68%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/examples/unicode\_test\_data.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvdW5pY29kZV90ZXN0X2RhdGEucHk=) | `22.00% <0.00%> (-78.00%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `65.62% <0.00%> (-9.38%)` | :arrow_down: |
| [superset/utils/decorators.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGVjb3JhdG9ycy5weQ==) | `50.76% <0.00%> (-4.24%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `82.14% <0.00%> (-3.58%)` | :arrow_down: |
| [superset/examples/helpers.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvaGVscGVycy5weQ==) | `95.00% <0.00%> (-2.50%)` | :arrow_down: |
| [superset/result\_set.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvcmVzdWx0X3NldC5weQ==) | `96.69% <0.00%> (-1.66%)` | :arrow_down: |
| [superset/models/sql\_lab.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL3NxbF9sYWIucHk=) | `90.69% <0.00%> (-0.65%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `81.42% <0.00%> (-0.50%)` | :arrow_down: |
| ... and [473 more](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=footer). Last update [d056e3d...55da74f](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter commented on pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#issuecomment-702807042
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=h1) Report
> Merging [#11131](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/d056e3dfd3a2dc4844f836979eb474abbbce4632?el=desc) will **decrease** coverage by `4.64%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #11131 +/- ##
==========================================
- Coverage 65.79% 61.14% -4.65%
==========================================
Files 816 826 +10
Lines 38422 39016 +594
Branches 3621 3667 +46
==========================================
- Hits 25280 23857 -1423
- Misses 13034 14978 +1944
- Partials 108 181 +73
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `62.21% <ø> (+0.44%)` | :arrow_up: |
| #python | `60.51% <ø> (-0.90%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/chart/ChartContainer.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L0NoYXJ0Q29udGFpbmVyLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupFormatters.js](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRm9ybWF0dGVycy5qcw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/reducers/index.js](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvcmVkdWNlcnMvaW5kZXguanM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupPluginsExtra.js](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwUGx1Z2luc0V4dHJhLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [235 more](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=footer). Last update [d056e3d...decdd9e](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kkucharc commented on a change in pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
kkucharc commented on a change in pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#discussion_r500473596
##########
File path: tests/fixtures/unicode_dashboard.py
##########
@@ -0,0 +1,104 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import pandas as pd
+import pytest
+from pandas import DataFrame
+from sqlalchemy import String
+from sqlalchemy.engine import Engine
+
+from superset import db
+from superset.connectors.sqla.models import SqlaTable
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+from superset.utils.core import get_example_database
+from tests.dashboard_utils import (
+ create_dashboard,
+ create_slice,
+ create_table_for_dashboard,
+)
+from tests.test_app import app
+
+
+@pytest.fixture()
+def load_unicode_dashboard_with_slice():
+ table_name = "unicode_test"
+ df = _get_dataframe()
+ with app.app_context():
+ yield _create_unicode_dashboard(df, table_name, "Unicode Cloud", None)
+
+ _cleanup()
Review comment:
Sure, you are right. I also added removing slice because apparently it doesn't cleanup after deleting particular dashboard record.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] dpgaspar commented on a change in pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#discussion_r498921390
##########
File path: tests/dashboard_utils.py
##########
@@ -0,0 +1,78 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# isort:skip_file
+"""Utils to provide dashboards for tests"""
+
+import json
+
+from pandas import DataFrame
+from typing import Dict, Any
+
+from superset import db, ConnectorRegistry
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+
+
+def create_table_for_dashboard(
+ df: DataFrame, tbl_name: str, database, schema: Dict[str, Any]
+):
+ df.to_sql(
+ tbl_name,
+ database.get_sqla_engine(),
+ if_exists="replace",
+ chunksize=500,
+ dtype=schema,
+ index=False,
+ method="multi",
+ )
+
+ tbl_source = ConnectorRegistry.sources["table"]
+ obj = db.session.query(tbl_source).filter_by(table_name=tbl_name).first()
+ if not obj:
+ obj = tbl_source(table_name=tbl_name)
+ obj.database = database
+ db.session.merge(obj)
+ db.session.commit()
+
+ return obj
+
+
+def create_slice(title, viz_type, tbl, slices_dict: Dict[str, str]):
+ return Slice(
+ slice_name=title,
+ viz_type=viz_type,
+ datasource_type="table",
+ datasource_id=tbl.id,
+ params=json.dumps(slices_dict, indent=4, sort_keys=True),
+ )
+
+
+def create_dashboard(slug: str, title: str, position: str, slc: Slice):
Review comment:
nit: missing return type
##########
File path: tests/dashboard_utils.py
##########
@@ -0,0 +1,78 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# isort:skip_file
+"""Utils to provide dashboards for tests"""
+
+import json
+
+from pandas import DataFrame
+from typing import Dict, Any
+
+from superset import db, ConnectorRegistry
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+
+
+def create_table_for_dashboard(
+ df: DataFrame, tbl_name: str, database, schema: Dict[str, Any]
+):
Review comment:
nit: missing return type
##########
File path: tests/dashboard_utils.py
##########
@@ -0,0 +1,78 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# isort:skip_file
+"""Utils to provide dashboards for tests"""
+
+import json
+
+from pandas import DataFrame
+from typing import Dict, Any
+
+from superset import db, ConnectorRegistry
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+
+
+def create_table_for_dashboard(
+ df: DataFrame, tbl_name: str, database, schema: Dict[str, Any]
+):
+ df.to_sql(
+ tbl_name,
+ database.get_sqla_engine(),
+ if_exists="replace",
+ chunksize=500,
+ dtype=schema,
+ index=False,
+ method="multi",
+ )
+
+ tbl_source = ConnectorRegistry.sources["table"]
+ obj = db.session.query(tbl_source).filter_by(table_name=tbl_name).first()
+ if not obj:
+ obj = tbl_source(table_name=tbl_name)
+ obj.database = database
+ db.session.merge(obj)
+ db.session.commit()
+
+ return obj
+
+
+def create_slice(title, viz_type, tbl, slices_dict: Dict[str, str]):
Review comment:
nit: missing return type
##########
File path: tests/security_tests.py
##########
@@ -1122,6 +1132,41 @@ def test_rls_filter_doesnt_alter_energy_query(self):
assert tbl.get_extra_cache_keys(self.query_obj) == []
assert "value > 1" not in sql
+ @pytest.fixture()
+ def load_unicode_dashboard(self):
+ data = [
+ {"phrase": "Под"},
+ {"phrase": "řšž"},
+ {"phrase": "視野無限廣"},
+ {"phrase": "微風"},
+ {"phrase": "中国智造"},
+ {"phrase": "æøå"},
+ {"phrase": "ëœéè"},
+ {"phrase": "いろはにほ"},
+ ]
+ tbl_name = "unicode_test"
+
+ # generate date/numeric data
+ df = pd.DataFrame.from_dict(data)
+
+ with self.create_app().app_context():
+ database = get_example_database()
+ schema = {
+ "phrase": String(500),
+ }
+ obj = create_table_for_dashboard(df, tbl_name, database, schema)
+ obj.fetch_metadata()
+
+ tbl = obj
+ slc = create_slice("Unicode Cloud", "word_cloud", tbl, None)
+ o = db.session.query(Slice).filter_by(slice_name=slc.slice_name).first()
+ if o:
+ db.session.delete(o)
+ db.session.add(slc)
+ db.session.commit()
+ create_dashboard("unicode-test", "Unicode Test", None, slc)
Review comment:
Yield and cleanup?
##########
File path: tests/databases/api_tests.py
##########
@@ -758,6 +768,38 @@ def test_test_connection_unsafe_uri(self):
}
self.assertEqual(response, expected_response)
+ @pytest.fixture()
+ def load_unicode_dashboard(self):
+ data = [
+ {"phrase": "Под"},
+ {"phrase": "řšž"},
+ {"phrase": "視野無限廣"},
+ {"phrase": "微風"},
+ {"phrase": "中国智造"},
+ {"phrase": "æøå"},
+ {"phrase": "ëœéè"},
+ {"phrase": "いろはにほ"},
+ ]
+ tbl_name = "unicode_test"
+
+ # generate date/numeric data
+ df = pd.DataFrame.from_dict(data)
+
+ with self.create_app().app_context():
+ database = get_example_database()
+ schema = {
+ "phrase": String(500),
+ "dttm": Date(),
+ "value": Float(),
+ }
+ obj = create_table_for_dashboard(df, tbl_name, database, schema)
+ obj.fetch_metadata()
+
+ db.session.commit()
+ position = "{}"
+ create_dashboard("unicode-test", "Unicode Test", position, None)
Review comment:
What's being done on other pytest fixtures is to `yield create_dashboard("unicode-test", "Unicode Test", position, None)` and then clean everything up (on this case delete the dashboard and then delete the table for dashboard)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kkucharc commented on a change in pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
kkucharc commented on a change in pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#discussion_r501022985
##########
File path: tests/dashboard_utils.py
##########
@@ -0,0 +1,83 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Utils to provide dashboards for tests"""
+
+import json
+from typing import Any, Dict
+
+from pandas import DataFrame
+
+from superset import ConnectorRegistry, db
+from superset.connectors.sqla.models import SqlaTable
+from superset.models.core import Database
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+
+
+def create_table_for_dashboard(
+ df: DataFrame, table_name: str, database: Database, schema: Dict[str, Any]
Review comment:
Sure, I'm appreciating any naming suggestions :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#discussion_r501040000
##########
File path: tests/dashboard_utils.py
##########
@@ -0,0 +1,83 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Utils to provide dashboards for tests"""
+
+import json
+from typing import Any, Dict
+
+from pandas import DataFrame
+
+from superset import ConnectorRegistry, db
+from superset.connectors.sqla.models import SqlaTable
+from superset.models.core import Database
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+
+
+def create_table_for_dashboard(
+ df: DataFrame, table_name: str, database: Database, dtype: Dict[str, Any]
+) -> SqlaTable:
+ df.to_sql(
+ table_name,
+ database.get_sqla_engine(),
+ if_exists="replace",
+ chunksize=500,
+ dtype=schema,
Review comment:
Sorry, I should have put this in my suggestion!
```suggestion
dtype=dtype,
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kkucharc commented on a change in pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
kkucharc commented on a change in pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#discussion_r500192297
##########
File path: tests/dashboard_utils.py
##########
@@ -0,0 +1,82 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Utils to provide dashboards for tests"""
+
+import json
+from typing import Any, Dict
+
+from pandas import DataFrame
+
+from superset import ConnectorRegistry, db
+from superset.connectors.sqla.models import SqlaTable
+from superset.models.core import Database
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+
+
+def create_table_for_dashboard(
+ df: DataFrame, tbl_name: str, database: Database, schema: Dict[str, Any]
Review comment:
I am also for more descriptive names instead of shortcuts. I changed it in both of my files.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kkucharc commented on pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
kkucharc commented on pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#issuecomment-704247237
@dpgaspar Thanks for review!
About moving file, I am not sure if putting this file in `tests/dashboards/` would be good idea, since `schedules_test.py` and `security_tests.py` which also use it are in `tests/` dir. Also actually `api_tests.py` which uses this fixture is in `databases/` - not in `dashboards/`. But it still creates dashboards for those tests, so I don't have really strong opinion about it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kkucharc commented on a change in pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
kkucharc commented on a change in pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#discussion_r499522989
##########
File path: tests/security_tests.py
##########
@@ -1122,6 +1132,41 @@ def test_rls_filter_doesnt_alter_energy_query(self):
assert tbl.get_extra_cache_keys(self.query_obj) == []
assert "value > 1" not in sql
+ @pytest.fixture()
+ def load_unicode_dashboard(self):
Review comment:
Sounds good. I just wanted to clarify if you mean moving to `fixtures/` directory? My only concern is about some fixtures that create slices for dashboards (or some other setup). I wondered if wouldn't be better to have just some common parts to combine those dashboard creation parts into particular fixtures (as there is this new file `dashboard_utils`. Or maybe having basic fixture which will be extended or decorated in more complex cases? WDYT @villebro ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#discussion_r499220155
##########
File path: tests/dashboard_utils.py
##########
@@ -0,0 +1,78 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# isort:skip_file
+"""Utils to provide dashboards for tests"""
+
+import json
+
+from pandas import DataFrame
+from typing import Dict, Any
+
+from superset import db, ConnectorRegistry
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+
+
+def create_table_for_dashboard(
+ df: DataFrame, tbl_name: str, database, schema: Dict[str, Any]
Review comment:
nit: `database: Database`. I assume this will cause a circular import, which we can get around by doing
```python
from typing import TYPE_CHECKING
if TYPE_CHECKING:
from superset.models.core import Database
def create_table_for_dashboard(
df: DataFrame, tbl_name: str, database: "Database", schema: Dict[str, Any]
):
```
The same can be applied to the return type.
##########
File path: tests/strategy_tests.py
##########
@@ -193,6 +204,39 @@ def reset_tag(self, tag):
db.session.delete(o)
db.session.commit()
+ @pytest.fixture()
+ def load_unicode_dashboard(self):
Review comment:
Same here (move to `fixtures.py`)
##########
File path: tests/security_tests.py
##########
@@ -1122,6 +1132,41 @@ def test_rls_filter_doesnt_alter_energy_query(self):
assert tbl.get_extra_cache_keys(self.query_obj) == []
assert "value > 1" not in sql
+ @pytest.fixture()
+ def load_unicode_dashboard(self):
Review comment:
Should we put this in `fixtures.py` so we don't have to repeat this logic wherever we use the unicode dashboard?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kkucharc commented on a change in pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
kkucharc commented on a change in pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#discussion_r499743101
##########
File path: tests/strategy_tests.py
##########
@@ -193,6 +204,39 @@ def reset_tag(self, tag):
db.session.delete(o)
db.session.commit()
+ @pytest.fixture()
+ def load_unicode_dashboard(self):
Review comment:
It should be moved now. Thanks for suggestion
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] dpgaspar commented on a change in pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#discussion_r500226761
##########
File path: tests/fixtures/unicode_dashboard.py
##########
@@ -0,0 +1,104 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import pandas as pd
+import pytest
+from pandas import DataFrame
+from sqlalchemy import String
+from sqlalchemy.engine import Engine
+
+from superset import db
+from superset.connectors.sqla.models import SqlaTable
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+from superset.utils.core import get_example_database
+from tests.dashboard_utils import (
+ create_dashboard,
+ create_slice,
+ create_table_for_dashboard,
+)
+from tests.test_app import app
+
+
+@pytest.fixture()
+def load_unicode_dashboard_with_slice():
+ table_name = "unicode_test"
+ df = _get_dataframe()
+ with app.app_context():
+ yield _create_unicode_dashboard(df, table_name, "Unicode Cloud", None)
+
+ _cleanup()
Review comment:
the `_cleanup()` is not actually cleaning up. The dashboard is still there. My idea is to make all test idempotent so we can run them multiple times and would help to clean weird behaviours (for example changing a test run order would make the test fail). So I think we should delete the dashboard also
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kkucharc commented on a change in pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
kkucharc commented on a change in pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#discussion_r499740099
##########
File path: tests/dashboard_utils.py
##########
@@ -0,0 +1,78 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# isort:skip_file
+"""Utils to provide dashboards for tests"""
+
+import json
+
+from pandas import DataFrame
+from typing import Dict, Any
+
+from superset import db, ConnectorRegistry
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+
+
+def create_table_for_dashboard(
+ df: DataFrame, tbl_name: str, database, schema: Dict[str, Any]
+):
+ df.to_sql(
+ tbl_name,
+ database.get_sqla_engine(),
+ if_exists="replace",
+ chunksize=500,
+ dtype=schema,
+ index=False,
+ method="multi",
+ )
+
+ tbl_source = ConnectorRegistry.sources["table"]
+ obj = db.session.query(tbl_source).filter_by(table_name=tbl_name).first()
+ if not obj:
+ obj = tbl_source(table_name=tbl_name)
+ obj.database = database
+ db.session.merge(obj)
+ db.session.commit()
+
+ return obj
+
+
+def create_slice(title, viz_type, tbl, slices_dict: Dict[str, str]):
+ return Slice(
+ slice_name=title,
+ viz_type=viz_type,
+ datasource_type="table",
+ datasource_id=tbl.id,
+ params=json.dumps(slices_dict, indent=4, sort_keys=True),
+ )
+
+
+def create_dashboard(slug: str, title: str, position: str, slc: Slice):
Review comment:
Fixed, thanks 👍
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kkucharc commented on a change in pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
kkucharc commented on a change in pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#discussion_r499742930
##########
File path: tests/security_tests.py
##########
@@ -1122,6 +1132,41 @@ def test_rls_filter_doesnt_alter_energy_query(self):
assert tbl.get_extra_cache_keys(self.query_obj) == []
assert "value > 1" not in sql
+ @pytest.fixture()
+ def load_unicode_dashboard(self):
Review comment:
That makes sens. I refactored this part. To be honest, I'm proud of this :) - I hope this will be good step on first iteration of this refactor. WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#discussion_r501014747
##########
File path: tests/dashboard_utils.py
##########
@@ -0,0 +1,83 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Utils to provide dashboards for tests"""
+
+import json
+from typing import Any, Dict
+
+from pandas import DataFrame
+
+from superset import ConnectorRegistry, db
+from superset.connectors.sqla.models import SqlaTable
+from superset.models.core import Database
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+
+
+def create_table_for_dashboard(
+ df: DataFrame, table_name: str, database: Database, schema: Dict[str, Any]
Review comment:
Not perhaps super important, but calling it `schema` here can be misleading, as `schema` typically refers to the database schema in the Superset backend context. Just to avoid confusion, I propose renaming this to `dtype` to align it with Pandas naming.
```suggestion
df: DataFrame, table_name: str, database: Database, dtype: Dict[str, Any]
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#discussion_r499683064
##########
File path: tests/dashboard_utils.py
##########
@@ -0,0 +1,78 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# isort:skip_file
+"""Utils to provide dashboards for tests"""
+
+import json
+
+from pandas import DataFrame
+from typing import Dict, Any
+
+from superset import db, ConnectorRegistry
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+
+
+def create_table_for_dashboard(
+ df: DataFrame, tbl_name: str, database, schema: Dict[str, Any]
Review comment:
No, if it works without it you should not use it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kkucharc commented on a change in pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
kkucharc commented on a change in pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#discussion_r499739475
##########
File path: tests/dashboard_utils.py
##########
@@ -0,0 +1,78 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# isort:skip_file
+"""Utils to provide dashboards for tests"""
+
+import json
+
+from pandas import DataFrame
+from typing import Dict, Any
+
+from superset import db, ConnectorRegistry
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+
+
+def create_table_for_dashboard(
+ df: DataFrame, tbl_name: str, database, schema: Dict[str, Any]
+):
Review comment:
Thanks for not noticing! Fixed :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io commented on pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#issuecomment-704954731
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@a071393`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #11131 +/- ##
=========================================
Coverage ? 62.33%
=========================================
Files ? 437
Lines ? 14628
Branches ? 3693
=========================================
Hits ? 9118
Misses ? 5329
Partials ? 181
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `62.33% <ø> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=footer). Last update [a071393...c4537ff](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kkucharc commented on pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
kkucharc commented on pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#issuecomment-704922956
I guess it was worth waiting for CI, because apparently cleanup had impact on charts tests. I also rebased because of conflicts. @villebro I think it's ready now :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#issuecomment-702807042
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=h1) Report
> Merging [#11131](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/d056e3dfd3a2dc4844f836979eb474abbbce4632?el=desc) will **decrease** coverage by `4.33%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #11131 +/- ##
==========================================
- Coverage 65.79% 61.45% -4.34%
==========================================
Files 816 826 +10
Lines 38422 39016 +594
Branches 3621 3667 +46
==========================================
- Hits 25280 23979 -1301
- Misses 13034 14856 +1822
- Partials 108 181 +73
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `62.21% <ø> (+0.44%)` | :arrow_up: |
| #python | `61.00% <ø> (-0.40%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/chart/ChartContainer.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L0NoYXJ0Q29udGFpbmVyLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupFormatters.js](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRm9ybWF0dGVycy5qcw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/reducers/index.js](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvcmVkdWNlcnMvaW5kZXguanM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupPluginsExtra.js](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwUGx1Z2luc0V4dHJhLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [232 more](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=footer). Last update [d056e3d...decdd9e](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#discussion_r499779535
##########
File path: tests/dashboard_utils.py
##########
@@ -0,0 +1,82 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Utils to provide dashboards for tests"""
+
+import json
+from typing import Any, Dict
+
+from pandas import DataFrame
+
+from superset import ConnectorRegistry, db
+from superset.connectors.sqla.models import SqlaTable
+from superset.models.core import Database
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+
+
+def create_table_for_dashboard(
+ df: DataFrame, tbl_name: str, database: Database, schema: Dict[str, Any]
+) -> SqlaTable:
+ df.to_sql(
+ tbl_name,
+ database.get_sqla_engine(),
+ if_exists="replace",
+ chunksize=500,
+ dtype=schema,
+ index=False,
+ method="multi",
+ )
+
+ tbl_source = ConnectorRegistry.sources["table"]
+ obj = db.session.query(tbl_source).filter_by(table_name=tbl_name).first()
+ if not obj:
+ obj = tbl_source(table_name=tbl_name)
+ obj.database = database
+ db.session.merge(obj)
+ db.session.commit()
+
+ return obj
+
+
+def create_slice(
+ title: str, viz_type: str, tbl: SqlaTable, slices_dict: Dict[str, str]
Review comment:
naming nit again
```suggestion
title: str, viz_type: str, table: SqlaTable, slices_dict: Dict[str, str]
```
##########
File path: tests/dashboard_utils.py
##########
@@ -0,0 +1,82 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Utils to provide dashboards for tests"""
+
+import json
+from typing import Any, Dict
+
+from pandas import DataFrame
+
+from superset import ConnectorRegistry, db
+from superset.connectors.sqla.models import SqlaTable
+from superset.models.core import Database
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+
+
+def create_table_for_dashboard(
+ df: DataFrame, tbl_name: str, database: Database, schema: Dict[str, Any]
+) -> SqlaTable:
+ df.to_sql(
+ tbl_name,
+ database.get_sqla_engine(),
+ if_exists="replace",
+ chunksize=500,
+ dtype=schema,
+ index=False,
+ method="multi",
+ )
+
+ tbl_source = ConnectorRegistry.sources["table"]
+ obj = db.session.query(tbl_source).filter_by(table_name=tbl_name).first()
+ if not obj:
+ obj = tbl_source(table_name=tbl_name)
+ obj.database = database
+ db.session.merge(obj)
+ db.session.commit()
+
+ return obj
+
+
+def create_slice(
+ title: str, viz_type: str, tbl: SqlaTable, slices_dict: Dict[str, str]
+) -> Slice:
+ return Slice(
+ slice_name=title,
+ viz_type=viz_type,
+ datasource_type="table",
+ datasource_id=tbl.id,
+ params=json.dumps(slices_dict, indent=4, sort_keys=True),
+ )
+
+
+def create_dashboard(slug: str, title: str, position: str, slc: Slice) -> Dashboard:
Review comment:
nit
```suggestion
def create_dashboard(slug: str, title: str, position: str, slice: Slice) -> Dashboard:
```
##########
File path: tests/fixtures/unicode_dashboard.py
##########
@@ -0,0 +1,106 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import pandas as pd
+import pytest
+from pandas import DataFrame
+from sqlalchemy import String
+from sqlalchemy.engine import Engine
+
+from superset import db
+from superset.connectors.sqla.models import SqlaTable
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+from superset.utils.core import get_example_database
+from tests.dashboard_utils import (
+ create_dashboard,
+ create_slice,
+ create_table_for_dashboard,
+)
+from tests.test_app import app
+
+
+@pytest.fixture()
+def load_unicode_dashboard_with_slice():
+ tbl_name = "unicode_test"
+ df = _get_dataframe()
+ with app.app_context():
+ yield _create_unicode_dashboard(df, tbl_name, "Unicode Cloud", None)
+
+ _cleanup()
+
+
+@pytest.fixture()
+def load_unicode_dashboard_with_position():
+ tbl_name = "unicode_test"
+ df = _get_dataframe()
+ position = "{}"
+ with app.app_context():
+ yield _create_unicode_dashboard(df, tbl_name, "Unicode Cloud", position)
+
+ _cleanup()
+
+
+def _get_dataframe():
+ data = _get_unicode_data()
+ return pd.DataFrame.from_dict(data)
+
+
+def _get_unicode_data():
+ return [
+ {"phrase": "Под"},
+ {"phrase": "řšž"},
+ {"phrase": "視野無限廣"},
+ {"phrase": "微風"},
+ {"phrase": "中国智造"},
+ {"phrase": "æøå"},
+ {"phrase": "ëœéè"},
+ {"phrase": "いろはにほ"},
+ ]
+
+
+def _create_unicode_dashboard(
+ df: DataFrame, tbl_name: str, slc_title: str, position: str
Review comment:
```suggestion
df: DataFrame, table_name: str, slice_title: str, position: str
```
##########
File path: tests/dashboard_utils.py
##########
@@ -0,0 +1,82 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Utils to provide dashboards for tests"""
+
+import json
+from typing import Any, Dict
+
+from pandas import DataFrame
+
+from superset import ConnectorRegistry, db
+from superset.connectors.sqla.models import SqlaTable
+from superset.models.core import Database
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+
+
+def create_table_for_dashboard(
+ df: DataFrame, tbl_name: str, database: Database, schema: Dict[str, Any]
Review comment:
nit: I personally find `table_name` more pythonic.
```suggestion
df: DataFrame, table_name: str, database: Database, schema: Dict[str, Any]
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] dpgaspar commented on a change in pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#discussion_r501025236
##########
File path: tests/fixtures/unicode_dashboard.py
##########
@@ -0,0 +1,110 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import pandas as pd
+import pytest
+from pandas import DataFrame
+from sqlalchemy import String
+
+from superset import db
+from superset.connectors.sqla.models import SqlaTable
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+from superset.utils.core import get_example_database
+from tests.dashboard_utils import (
+ create_dashboard,
+ create_slice,
+ create_table_for_dashboard,
+)
+from tests.test_app import app
+
+
+@pytest.fixture()
+def load_unicode_dashboard_with_slice():
+ table_name = "unicode_test"
+ slice_name = "Unicode Cloud"
+ df = _get_dataframe()
+ with app.app_context():
+ dash = _create_unicode_dashboard(df, table_name, slice_name, None)
+ yield
+
+ _cleanup(dash, slice_name)
+
+
+@pytest.fixture()
+def load_unicode_dashboard_with_position():
+ table_name = "unicode_test"
+ slice_name = "Unicode Cloud"
+ df = _get_dataframe()
+ position = "{}"
+ with app.app_context():
+ dash = _create_unicode_dashboard(df, table_name, slice_name, position)
+ yield
+ _cleanup(dash, slice_name)
+
+
+def _get_dataframe():
+ data = _get_unicode_data()
+ return pd.DataFrame.from_dict(data)
+
+
+def _get_unicode_data():
+ return [
+ {"phrase": "Под"},
+ {"phrase": "řšž"},
+ {"phrase": "視野無限廣"},
+ {"phrase": "微風"},
+ {"phrase": "中国智造"},
+ {"phrase": "æøå"},
+ {"phrase": "ëœéè"},
+ {"phrase": "いろはにほ"},
+ ]
+
+
+def _create_unicode_dashboard(
+ df: DataFrame, table_name: str, slice_title: str, position: str
+) -> Dashboard:
+ database = get_example_database()
+ schema = {
+ "phrase": String(500),
+ }
+ table = create_table_for_dashboard(df, table_name, database, schema)
+ table.fetch_metadata()
+
+ if slice_title:
+ slice = _create_and_commit_unicode_slice(table, slice_title)
+
+ return create_dashboard("unicode-test", "Unicode Test", position, slice)
+
+
+def _create_and_commit_unicode_slice(table: SqlaTable, title: str):
+ slice = create_slice(title, "word_cloud", table, {})
+ o = db.session.query(Slice).filter_by(slice_name=slice.slice_name).first()
+ if o:
+ db.session.delete(o)
+ db.session.add(slice)
+ db.session.commit()
+ return slice
+
+
+def _cleanup(dash: Dashboard, slice_name: str) -> None:
+ engine = get_example_database().get_sqla_engine()
+ engine.execute("DROP TABLE IF EXISTS unicode_test")
+ db.session.delete(dash)
+ if slice_name:
+ slice = db.session.query(Slice).filter_by(slice_name=slice_name).first()
Review comment:
nit: slice = db.session.query(Slice).filter_by(slice_name=slice_name).one_or_none()
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#discussion_r499608860
##########
File path: tests/security_tests.py
##########
@@ -1122,6 +1132,41 @@ def test_rls_filter_doesnt_alter_energy_query(self):
assert tbl.get_extra_cache_keys(self.query_obj) == []
assert "value > 1" not in sql
+ @pytest.fixture()
+ def load_unicode_dashboard(self):
Review comment:
@kkucharc you are right, I meant the `fixtures/` directory! Perhaps we could create something like `fixtures/unicode.py` that would for now just include `def load_unicode_dashboard()`. Down the road I actually think it would be great to try to decouple datasets, charts and dashboards from each other, as may backend tests don't necessarily need the full suite of charts and dashboards, but are ok with just the dataset. Having said that, let's take gradual steps here, so as to not choke on trying to make this perfect on first iteration.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kkucharc commented on a change in pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
kkucharc commented on a change in pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#discussion_r499741514
##########
File path: tests/dashboard_utils.py
##########
@@ -0,0 +1,78 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# isort:skip_file
+"""Utils to provide dashboards for tests"""
+
+import json
+
+from pandas import DataFrame
+from typing import Dict, Any
+
+from superset import db, ConnectorRegistry
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+
+
+def create_table_for_dashboard(
+ df: DataFrame, tbl_name: str, database, schema: Dict[str, Any]
Review comment:
It should be ok. Thanks for suggestion.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
villebro commented on pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#issuecomment-704715653
@kkucharc do you prefer getting review comments now or wait until CI is passing?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kkucharc commented on a change in pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
kkucharc commented on a change in pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#discussion_r499739888
##########
File path: tests/dashboard_utils.py
##########
@@ -0,0 +1,78 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# isort:skip_file
+"""Utils to provide dashboards for tests"""
+
+import json
+
+from pandas import DataFrame
+from typing import Dict, Any
+
+from superset import db, ConnectorRegistry
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+
+
+def create_table_for_dashboard(
+ df: DataFrame, tbl_name: str, database, schema: Dict[str, Any]
+):
+ df.to_sql(
+ tbl_name,
+ database.get_sqla_engine(),
+ if_exists="replace",
+ chunksize=500,
+ dtype=schema,
+ index=False,
+ method="multi",
+ )
+
+ tbl_source = ConnectorRegistry.sources["table"]
+ obj = db.session.query(tbl_source).filter_by(table_name=tbl_name).first()
+ if not obj:
+ obj = tbl_source(table_name=tbl_name)
+ obj.database = database
+ db.session.merge(obj)
+ db.session.commit()
+
+ return obj
+
+
+def create_slice(title, viz_type, tbl, slices_dict: Dict[str, str]):
Review comment:
Fixed, thanks 👍
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kkucharc commented on a change in pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
kkucharc commented on a change in pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#discussion_r499740629
##########
File path: tests/databases/api_tests.py
##########
@@ -758,6 +768,38 @@ def test_test_connection_unsafe_uri(self):
}
self.assertEqual(response, expected_response)
+ @pytest.fixture()
+ def load_unicode_dashboard(self):
+ data = [
+ {"phrase": "Под"},
+ {"phrase": "řšž"},
+ {"phrase": "視野無限廣"},
+ {"phrase": "微風"},
+ {"phrase": "中国智造"},
+ {"phrase": "æøå"},
+ {"phrase": "ëœéè"},
+ {"phrase": "いろはにほ"},
+ ]
+ tbl_name = "unicode_test"
+
+ # generate date/numeric data
+ df = pd.DataFrame.from_dict(data)
+
+ with self.create_app().app_context():
+ database = get_example_database()
+ schema = {
+ "phrase": String(500),
+ "dttm": Date(),
+ "value": Float(),
+ }
+ obj = create_table_for_dashboard(df, tbl_name, database, schema)
+ obj.fetch_metadata()
+
+ db.session.commit()
+ position = "{}"
+ create_dashboard("unicode-test", "Unicode Test", position, None)
Review comment:
Thanks for the hint. I changed it this way. Thanks to that I also removed deleting dashboard from `conftest.py`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro merged pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
villebro merged pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#issuecomment-702807042
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=h1) Report
> Merging [#11131](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/d056e3dfd3a2dc4844f836979eb474abbbce4632?el=desc) will **decrease** coverage by `4.33%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #11131 +/- ##
==========================================
- Coverage 65.79% 61.45% -4.34%
==========================================
Files 816 826 +10
Lines 38422 39016 +594
Branches 3621 3667 +46
==========================================
- Hits 25280 23978 -1302
- Misses 13034 14857 +1823
- Partials 108 181 +73
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `62.21% <ø> (+0.44%)` | :arrow_up: |
| #python | `61.00% <ø> (-0.41%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvaW5kZXguanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/chart/ChartContainer.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L0NoYXJ0Q29udGFpbmVyLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupFormatters.js](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRm9ybWF0dGVycy5qcw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/reducers/index.js](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvcmVkdWNlcnMvaW5kZXguanM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupPluginsExtra.js](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwUGx1Z2luc0V4dHJhLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [233 more](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=footer). Last update [d056e3d...decdd9e](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#issuecomment-704954731
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@a071393`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #11131 +/- ##
=========================================
Coverage ? 73.57%
=========================================
Files ? 437
Lines ? 14631
Branches ? 3693
=========================================
Hits ? 10765
Misses ? 3759
Partials ? 107
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `55.97% <ø> (?)` | |
| #javascript | `62.33% <ø> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=footer). Last update [a071393...c4537ff](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#discussion_r499782250
##########
File path: tests/security_tests.py
##########
@@ -1122,6 +1132,41 @@ def test_rls_filter_doesnt_alter_energy_query(self):
assert tbl.get_extra_cache_keys(self.query_obj) == []
assert "value > 1" not in sql
+ @pytest.fixture()
+ def load_unicode_dashboard(self):
Review comment:
Yeah, I really like this refactor! Hi-5 @kkucharc ! 💯
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kkucharc commented on a change in pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
kkucharc commented on a change in pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#discussion_r499602959
##########
File path: tests/dashboard_utils.py
##########
@@ -0,0 +1,78 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# isort:skip_file
+"""Utils to provide dashboards for tests"""
+
+import json
+
+from pandas import DataFrame
+from typing import Dict, Any
+
+from superset import db, ConnectorRegistry
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+
+
+def create_table_for_dashboard(
+ df: DataFrame, tbl_name: str, database, schema: Dict[str, Any]
Review comment:
Sure, thanks for spotting that. If there is no problem with circular import while running tests, do you think it still might need `TYPE_CHECKING`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#issuecomment-704954731
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@a071393`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #11131 +/- ##
=========================================
Coverage ? 55.99%
=========================================
Files ? 403
Lines ? 13362
Branches ? 3429
=========================================
Hits ? 7482
Misses ? 5696
Partials ? 184
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `55.99% <ø> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=footer). Last update [a071393...7c78f7a](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kkucharc commented on a change in pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
kkucharc commented on a change in pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#discussion_r499740961
##########
File path: tests/security_tests.py
##########
@@ -1122,6 +1132,41 @@ def test_rls_filter_doesnt_alter_energy_query(self):
assert tbl.get_extra_cache_keys(self.query_obj) == []
assert "value > 1" not in sql
+ @pytest.fixture()
+ def load_unicode_dashboard(self):
+ data = [
+ {"phrase": "Под"},
+ {"phrase": "řšž"},
+ {"phrase": "視野無限廣"},
+ {"phrase": "微風"},
+ {"phrase": "中国智造"},
+ {"phrase": "æøå"},
+ {"phrase": "ëœéè"},
+ {"phrase": "いろはにほ"},
+ ]
+ tbl_name = "unicode_test"
+
+ # generate date/numeric data
+ df = pd.DataFrame.from_dict(data)
+
+ with self.create_app().app_context():
+ database = get_example_database()
+ schema = {
+ "phrase": String(500),
+ }
+ obj = create_table_for_dashboard(df, tbl_name, database, schema)
+ obj.fetch_metadata()
+
+ tbl = obj
+ slc = create_slice("Unicode Cloud", "word_cloud", tbl, None)
+ o = db.session.query(Slice).filter_by(slice_name=slc.slice_name).first()
+ if o:
+ db.session.delete(o)
+ db.session.add(slc)
+ db.session.commit()
+ create_dashboard("unicode-test", "Unicode Test", None, slc)
Review comment:
I refactored this part to separate fixture and I put there both. Thanks for suggestion!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#issuecomment-702807042
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=h1) Report
> Merging [#11131](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/d056e3dfd3a2dc4844f836979eb474abbbce4632?el=desc) will **decrease** coverage by `5.32%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #11131 +/- ##
==========================================
- Coverage 65.79% 60.46% -5.33%
==========================================
Files 816 391 -425
Lines 38422 24472 -13950
Branches 3621 0 -3621
==========================================
- Hits 25280 14798 -10482
+ Misses 13034 9674 -3360
+ Partials 108 0 -108
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `?` | |
| #python | `60.46% <ø> (-0.94%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/examples/unicode\_test\_data.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvdW5pY29kZV90ZXN0X2RhdGEucHk=) | `22.00% <0.00%> (-78.00%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `69.76% <0.00%> (-12.16%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `65.62% <0.00%> (-9.38%)` | :arrow_down: |
| [superset/utils/decorators.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGVjb3JhdG9ycy5weQ==) | `50.76% <0.00%> (-4.24%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `82.14% <0.00%> (-3.58%)` | :arrow_down: |
| [superset/examples/world\_bank.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvd29ybGRfYmFuay5weQ==) | `97.10% <0.00%> (-2.90%)` | :arrow_down: |
| [superset/examples/birth\_names.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvYmlydGhfbmFtZXMucHk=) | `97.36% <0.00%> (-2.64%)` | :arrow_down: |
| [superset/examples/helpers.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvaGVscGVycy5weQ==) | `95.00% <0.00%> (-2.50%)` | :arrow_down: |
| ... and [480 more](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=footer). Last update [d056e3d...55da74f](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] willbarrett commented on pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
willbarrett commented on pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#issuecomment-702822739
@villebro @dpgaspar this is the first PR of a refactor to move away from the example data and to Pytest Fixtures. Please help validate the approach before we cross-apply this type of change to the rest of the suite.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] dpgaspar commented on a change in pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#discussion_r501025236
##########
File path: tests/fixtures/unicode_dashboard.py
##########
@@ -0,0 +1,110 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import pandas as pd
+import pytest
+from pandas import DataFrame
+from sqlalchemy import String
+
+from superset import db
+from superset.connectors.sqla.models import SqlaTable
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+from superset.utils.core import get_example_database
+from tests.dashboard_utils import (
+ create_dashboard,
+ create_slice,
+ create_table_for_dashboard,
+)
+from tests.test_app import app
+
+
+@pytest.fixture()
+def load_unicode_dashboard_with_slice():
+ table_name = "unicode_test"
+ slice_name = "Unicode Cloud"
+ df = _get_dataframe()
+ with app.app_context():
+ dash = _create_unicode_dashboard(df, table_name, slice_name, None)
+ yield
+
+ _cleanup(dash, slice_name)
+
+
+@pytest.fixture()
+def load_unicode_dashboard_with_position():
+ table_name = "unicode_test"
+ slice_name = "Unicode Cloud"
+ df = _get_dataframe()
+ position = "{}"
+ with app.app_context():
+ dash = _create_unicode_dashboard(df, table_name, slice_name, position)
+ yield
+ _cleanup(dash, slice_name)
+
+
+def _get_dataframe():
+ data = _get_unicode_data()
+ return pd.DataFrame.from_dict(data)
+
+
+def _get_unicode_data():
+ return [
+ {"phrase": "Под"},
+ {"phrase": "řšž"},
+ {"phrase": "視野無限廣"},
+ {"phrase": "微風"},
+ {"phrase": "中国智造"},
+ {"phrase": "æøå"},
+ {"phrase": "ëœéè"},
+ {"phrase": "いろはにほ"},
+ ]
+
+
+def _create_unicode_dashboard(
+ df: DataFrame, table_name: str, slice_title: str, position: str
+) -> Dashboard:
+ database = get_example_database()
+ schema = {
+ "phrase": String(500),
+ }
+ table = create_table_for_dashboard(df, table_name, database, schema)
+ table.fetch_metadata()
+
+ if slice_title:
+ slice = _create_and_commit_unicode_slice(table, slice_title)
+
+ return create_dashboard("unicode-test", "Unicode Test", position, slice)
+
+
+def _create_and_commit_unicode_slice(table: SqlaTable, title: str):
+ slice = create_slice(title, "word_cloud", table, {})
+ o = db.session.query(Slice).filter_by(slice_name=slice.slice_name).first()
+ if o:
+ db.session.delete(o)
+ db.session.add(slice)
+ db.session.commit()
+ return slice
+
+
+def _cleanup(dash: Dashboard, slice_name: str) -> None:
+ engine = get_example_database().get_sqla_engine()
+ engine.execute("DROP TABLE IF EXISTS unicode_test")
+ db.session.delete(dash)
+ if slice_name:
+ slice = db.session.query(Slice).filter_by(slice_name=slice_name).first()
Review comment:
nit: `slice = db.session.query(Slice).filter_by(slice_name=slice_name).one_or_none()`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-commenter edited a comment on pull request #11131: test: removed unicode_test example from unit tests
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #11131:
URL: https://github.com/apache/incubator-superset/pull/11131#issuecomment-702807042
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=h1) Report
> Merging [#11131](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/d056e3dfd3a2dc4844f836979eb474abbbce4632?el=desc) will **decrease** coverage by `5.03%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #11131 +/- ##
==========================================
- Coverage 65.79% 60.75% -5.04%
==========================================
Files 816 391 -425
Lines 38422 24483 -13939
Branches 3621 0 -3621
==========================================
- Hits 25280 14875 -10405
+ Misses 13034 9608 -3426
+ Partials 108 0 -108
```
| Flag | Coverage Δ | |
|---|---|---|
| #cypress | `?` | |
| #javascript | `?` | |
| #python | `60.75% <ø> (-0.66%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/examples/unicode\_test\_data.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvdW5pY29kZV90ZXN0X2RhdGEucHk=) | `22.00% <0.00%> (-78.00%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
| [superset/utils/decorators.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGVjb3JhdG9ycy5weQ==) | `50.76% <0.00%> (-4.24%)` | :arrow_down: |
| [superset/examples/helpers.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZXhhbXBsZXMvaGVscGVycy5weQ==) | `95.00% <0.00%> (-2.50%)` | :arrow_down: |
| [superset/models/sql\_lab.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL3NxbF9sYWIucHk=) | `90.69% <0.00%> (-0.65%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `81.42% <0.00%> (-0.50%)` | :arrow_down: |
| [superset/utils/pandas\_postprocessing.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvcGFuZGFzX3Bvc3Rwcm9jZXNzaW5nLnB5) | `76.47% <0.00%> (-0.50%)` | :arrow_down: |
| [superset/views/base\_api.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvYmFzZV9hcGkucHk=) | `97.90% <0.00%> (-0.36%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.12% <0.00%> (-0.28%)` | :arrow_down: |
| ... and [470 more](https://codecov.io/gh/apache/incubator-superset/pull/11131/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=footer). Last update [d056e3d...55da74f](https://codecov.io/gh/apache/incubator-superset/pull/11131?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org