test(s3): mock AWS client in close-with-session test - #4442
Merged
Jan-Kazlouski-elastic merged 2 commits intoSep 4, 2026
Merged
Conversation
Stop test_close_with_client_session from making real ListBuckets calls. Assert close() tears down the AsyncExitStack instead. Co-authored-by: Cursor <cursoragent@cursor.com>
erikcurrin-elastic
approved these changes
Sep 4, 2026
Jan-Kazlouski-elastic
deleted the
test/s3-mock-close-with-client-session
branch
September 4, 2026 18:45
This was referenced Sep 4, 2026
12 tasks
Jan-Kazlouski-elastic
added a commit
that referenced
this pull request
Sep 9, 2026
<!--Provide a general description of the code changes in your pull request. If the change relates to a specific issue, include the link at the top. If this is an ad-hoc/trivial change and does not have a corresponding issue, please describe your changes in enough details, so that reviewers and other team members can understand the reasoning behind the pull request.--> Several unit tests passed assertions but intermittently failed CI under the default 1s `pytest-fail-slow` threshold because they still hit real I/O: - Oracle/MSSQL tests patched `Engine.connect` but still called real `create_engine` - GraphQL `close()` accessed the cached `session` property, creating an `aiohttp.ClientSession` even when none was used - Some GitLab `get_docs` integration tests left `get_work_items_group` unmocked, triggering real HTTP for epic fetches This PR mocks `create_engine` via 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 <!--You can remove unrelated items from checklists below and/or add new items that may help during the review.--> #### Pre-Review Checklist - [x] this PR does NOT contain credentials of any kind, such as API keys or username/passwords (double check `config.yml.example`) - [x] this PR has a meaningful title - [ ] this PR links to all relevant github issues that it fixes or partially addresses - [x] if there is no GH issue, please create it. Each PR should have a link to an issue - [x] this PR has a thorough description - [x] Covered the changes with automated tests - [x] Tested the changes locally - [x] Added a label for each target release version (example: `v7.13.2`, `v7.14.0`, `v8.0.0`) - [ ] For bugfixes: backport safely to all minor branches still receiving patch releases - [ ] Considered corresponding documentation changes - [ ] Contributed any configuration settings changes to the configuration reference - [ ] if you added or changed Rich Configurable Fields for a Native Connector, you made a corresponding PR in [Kibana](https://github.com/elastic/kibana/blob/main/packages/kbn-search-connectors/types/native_connectors.ts) #### Changes Requiring Extra Attention <!--Please call out any changes that require special attention from the reviewers and/or increase the risk to availability or security of the system after deployment. Remove the ones that don't apply.--> ## Related Pull Requests * #4184 * #4442 ## Release Note <!--If you think this enhancement/fix should be included in the release notes, please write a concise user-facing description of the change here. You should also label the PR with `release_note` so the release notes author(s) can easily look it up.-->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
test_close_with_client_sessionwas making real AWSListBucketsHTTP calls with dummy credentials. That caused slow, flaky failures under--fail-slow=1when network latency exceeded 1s.The test now mocks the S3 client (same pattern as
test_ping) and asserts thatclose()tears down theAsyncExitStackused for the client session.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
n/a — test-only change.
Made with Cursor