Skip to content
This repository was archived by the owner on Mar 23, 2026. It is now read-only.

tests: Fix pre-install fixtures logic#13709

Merged
bentsku merged 3 commits intomainfrom
fix-fixture-cleanup
Feb 6, 2026
Merged

tests: Fix pre-install fixtures logic#13709
bentsku merged 3 commits intomainfrom
fix-fixture-cleanup

Conversation

@bentsku
Copy link
Copy Markdown
Contributor

@bentsku bentsku commented Feb 6, 2026 •
edited
Loading

Motivation

Follow up from #13701, the fixture logic was wrong and I could not use early returns, because they were yield fixtures, aka generators. See https://docs.pytest.org/en/stable/how-to/fixtures.html#yield-fixtures-recommended

I've updated the fixture to not use yield anymore, allowing me to use early returns which makes more sense 👍

I've now tested properly the change and it works with no errors 👍 thanks @nik-localstack for surfacing this so quickly!

We got the following error before:

ValueError: opensearch did not yield a value

Changes

  • update the type of the fixtures by remove the yield as they do not have any clean up logic, so that can use early return

Tests

Start LocalStack in an external container and run a test from each service like tests.aws.services.opensearch.test_opensearch.TestOpensearchProvider.test_list_versions and see that it works 👍

Related

@bentsku bentsku added this to the 4.14 milestone Feb 6, 2026
@bentsku bentsku self-assigned this Feb 6, 2026
@bentsku bentsku requested a review from silv-io as a code owner February 6, 2026 10:42
@bentsku bentsku added the semver: patch Non-breaking changes which can be included in patch releases label Feb 6, 2026
@bentsku bentsku added docs: skip Pull request does not require documentation changes notes: skip Pull request does not have to be mentioned in the release notes labels Feb 6, 2026
k-a-il
k-a-il approved these changes Feb 6, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@k-a-il k-a-il left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to learn that you could not return from it, nice catch. LGTM

@bentsku
Copy link
Copy Markdown
Contributor Author

bentsku commented Feb 6, 2026 •
edited
Loading

Good to learn that you could not return from it, nice catch. LGTM

Now that you say it, I looked it up a bit: https://docs.pytest.org/en/stable/how-to/fixtures.html#yield-fixtures-recommended

I actually could swap the yield and make it a return fixture instead and it would work 😅 I'll update, seems cleaner, as the fixtures do not have clean up logic

@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 6, 2026

LocalStack Community integration with Pro

 2 files  ±    0   2 suites  ±0   10m 49s ⏱️ - 1h 47m 49s
73 tests  - 5 123  56 ✅  - 4 789  17 💤  - 334  0 ❌ ±0 
75 runs   - 5 123  56 ✅  - 4 789  19 💤  - 334  0 ❌ ±0 

Results for commit 44deaf2. ± Comparison against base commit 965a755.

This pull request removes 5123 tests. 
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 6, 2026

Test Results - Preflight, Unit

23 114 tests  ±0   21 255 ✅ ±0   6m 16s ⏱️ -1s
     1 suites ±0    1 859 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 44deaf2. ± Comparison against base commit 965a755.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 6, 2026

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 4s ⏱️ -3s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 44deaf2. ± Comparison against base commit 965a755.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 6, 2026

Test Results (amd64) - Integration, Bootstrap

  5 files   5 suites   20m 19s ⏱️
 97 tests 80 ✅ 17 💤 0 ❌
103 runs  80 ✅ 23 💤 0 ❌

Results for commit 44deaf2.

@bentsku bentsku merged commit 55611d1 into main Feb 6, 2026
42 checks passed
@bentsku bentsku deleted the fix-fixture-cleanup branch February 6, 2026 11:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@silv-io silv-io Awaiting requested review from silv-io

@alexrashed alexrashed Awaiting requested review from alexrashed alexrashed is a code owner

1 more reviewer

@k-a-il k-a-il k-a-il approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@bentsku bentsku

Labels

docs: skip Pull request does not require documentation changes notes: skip Pull request does not have to be mentioned in the release notes semver: patch Non-breaking changes which can be included in patch releases

Projects

None yet

Milestone

4.14

Development

Successfully merging this pull request may close these issues.

2 participants

@bentsku @k-a-il