sadpandajoe commented on code in PR #43303:
URL: https://github.com/apache/superset/pull/43303#discussion_r4445255414


##########
tests/integration_tests/charts/data/api_tests.py:
##########
@@ -1269,6 +1270,161 @@ def test_chart_data_get(self):
         assert data["result"][0]["status"] == "success"
         assert data["result"][0]["rowcount"] == 2
 
+    # ------------------------------------------------------------------ #
+    # #33615 — synthesized query_context read-path verification.
+    #
+    # These exercise the REAL read/authz/RLS path (get_data →
+    # ChartDataCommand.validate() → raise_for_access) on a chart whose
+    # query_context was produced by `build_query_context_config` — the same
+    # helper the importer runs (ADR-013/014). The verifier is NOT mocked
+    # (SECURITY-CRITICAL discipline). They are the api-side counterpart of the
+    # importer round-trip (F3-T1), baseline-equivalence (F3-T2), and security
+    # (F3-T3) tasks. The existing 68 tests in this module stand unmodified as
+    # the NFR-COMPAT-01 regression guard.
+    # ------------------------------------------------------------------ #
+
+    #: params equivalent to the "Genders" chart's saved viz form-data.
+    _GENDERS_PARAMS = {
+        "metrics": ["sum__num"],
+        "groupby": ["gender"],
+        "time_range": "1900-01-01T00:00:00 : 2000-01-01T00:00:00",
+        "granularity_sqla": "ds",
+        "row_limit": 50000,
+        "order_desc": True,
+    }
+
+    @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+    def test_synthesized_query_context_returns_data(self):

Review Comment:
   Moving them into an enabled class is the fix: any test class in 
`tests/integration_tests/charts/data/api_tests.py` without the class-level skip 
(or a new class in that module) keeps the four read-path tests in the same file 
and they actually run. Until then, none of them can fail if a synthesized 
context is rejected by `/data/`, and `commands_tests.py` only checks 
persistence, not the read path. Could they go into a non-skipped class there, 
asserting a 200 and rows for an imported chart?



-- 
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]

Reply via email to