test: mock real I/O in unit tests to stop fail-slow flakes - #4455
Conversation
Skip creating aiohttp sessions on GraphQL close when none was opened. Mock SQLAlchemy create_engine in Oracle/MSSQL tests instead of only patching Engine.connect. Stub GitLab epic fetches in get_docs tests that do not exercise epics.
| self._sleeps.cancel() | ||
| await self.session.close() | ||
| del self.session | ||
| session = self.__dict__.get("session") |
There was a problem hiding this comment.
That doesn't look right. Why?
There was a problem hiding this comment.
__dict__.get("session") approach was done to work around how @cached_property behaves. Reading self.session in close() always creates an aiohttp session, even when the test never made a request. Our tests always call close() on teardown, so that was triggering real session creation and flaky fail-slow failures.
The idea was to guard close() in production so it only shuts down a session if one was already opened. That works in practice, but I agree that it looks weird and we probably wouldn't want this in production code as part of test fix only PR.
I've switched to a different fix instead: create_graphql_source now mocks aiohttp.ClientSession, so teardown stays fast without changing connector code. The production close() is back to what it was on main.
We might want to clean up session handling in the GraphQL client itself (with explicit _session = None like Confluence/Jira) a as a follow up.
…close() Revert the production close() guard and patch aiohttp.ClientSession in create_graphql_source so teardown stays fast without touching connector code.
|
@artem-shelkovnikov |
) (#4468) Backports #4455 to `8.19`. ## Summary - Add shared `mock_sync_db_engine` helper and use it in Oracle/MSSQL tests instead of only patching `Engine.connect`. - Mock `aiohttp.ClientSession` in the GraphQL test helper so teardown does not open a real session. - Add regression test asserting GraphQL teardown does not instantiate a real `ClientSession`. ## Backport notes - The 8.19 branch uses monolithic `connectors/sources/{graphql,mssql,oracle}.py` modules, so patch targets differ from `main` (`connectors.sources.graphql.aiohttp.ClientSession`, `connectors.sources.mssql.create_engine`, etc.). - GitLab `get_work_items_group` stubs from the original PR are not applicable on 8.19 (method does not exist there). - The incidental `NOTICE.txt` version bump from `make install` is intentionally excluded. ## Test plan - [x] `make clean install autoformat lint test` - [x] `pytest tests/sources/test_graphql.py tests/sources/test_mssql.py tests/sources/test_oracle.py`
2 participants
Several unit tests passed assertions but intermittently failed CI under the default 1s
pytest-fail-slowthreshold because they still hit real I/O:Engine.connectbut still called realcreate_engineclose()accessed the cachedsessionproperty, creating anaiohttp.ClientSessioneven when none was usedget_docsintegration tests leftget_work_items_groupunmocked, triggering real HTTP for epic fetchesThis PR mocks
create_enginevia a shared test helper, guards GraphQL session teardown, stubs epic fetches where not under test, and adds a regression test for GraphQL close behavior.Checklists
Pre-Review Checklist
config.yml.example)v7.13.2,v7.14.0,v8.0.0)Changes Requiring Extra Attention
Related Pull Requests
Release Note