henry3260 commented on PR #70114: URL: https://github.com/apache/airflow/pull/70114#issuecomment-5019578038
> I wonder whether we should delete this test entirely, as it tests an internal behavior: > > * The owned fact: `_test_connection_sql == "SELECT 1"` is already pinned directly by `test_test_connection_sql`. > * The behavior - "test_connection() succeeds end-to-end" is already pinned by `test_test_connection_success`. > * The current test's only distinct claim is "the literal SQL reaches the driver," which is unobservable without reaching into the internals (client.query). > > What do you think? Thanks for laying it out — I'd lean toward keeping it, tightened along the lines Wei suggested. Per Wei's point, the first two positional arguments are ours — the SQL and the query parameters — so asserting on those is asserting our own contract, not the driver's. does that make sense to you? -- 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]
