rusackas commented on PR #43020:
URL: https://github.com/apache/superset/pull/43020#issuecomment-5647585337
Got you an actual answer on the `validate_sql` coverage question. The old
Postgres path (`ecpg`/`pgsanity`) was a pure syntax checker, no live
connection, so it rode along for free in `test-postgres` just because that
job's example db happens to report backend `postgresql`. `PrestoDBSQLValidator`
is different: it sends `EXPLAIN (TYPE VALIDATE) <sql>` to a real Presto
connection, so removing Postgres and keeping only Presto isn't a like-for-like
swap, it actually needs a job that provisions a reachable Presto server.
Good news: that job already exists. `test-postgres-presto` already spins up
a live Presto container and points `SUPERSET__SQLALCHEMY_EXAMPLES_URI` at it,
but it selects tests with `-m 'chart_data_flow or sql_json_flow'`, and none of
the four remaining `validate_sql` tests carry either marker, so they're never
even collected there, independent of the `example_db.backend != "presto"`
runtime skip. So this isn't "we need new CI infra", it's a missing marker on
tests whose infra is already sitting right there unused.
Recommend adding `@pytest.mark.sql_json_flow` (matches `TestSqlLab`'s
existing convention in `sqllab_tests.py`) as the outermost decorator on the
four methods, so they land in the class but not the whole class-level mark
(that class has a lot of unrelated tests):
```python
@pytest.mark.sql_json_flow
@mock.patch.dict(
"superset.config.SQL_VALIDATORS_BY_ENGINE",
SQL_VALIDATORS_BY_ENGINE,
clear=True,
)
def test_validate_sql(self):
...
```
Same one-line addition above the `@mock.patch.dict(...)` on
`test_validate_sql_with_jinja_templates`,
`test_validate_sql_with_jinja_templates_and_params`, and
`test_validate_sql_with_jinja_invalid_sql_after_render`.
Separately: GitHub's still showing conflicts, needs a rebase before this can
merge.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
