Refactor: Events: Clean up tests #10160

maxhoheiser · 2024-02-01T17:54:45Z

Motivation

The current events tests are all defined in a single file .../events/test_events.py, including (except one) the fixtures not imported from ...fixtures.py. This makes it hard to read and add new tests especially grouping them to similar tests.

Change

extracted events tests specific fixtures into conftest.py file
separate tests targeting EventBus into a separate class
extracted events tests targeting rule testing into separate file tests_events_rules.py
extracted events tests specifically testing integration with additional services into separate file tests_events_integrations.py
extracted events testing input transformation e.g. InputPaths (and future InputTransformers) in to separate file tests_events_inputs.py, also separating InputPaths into a separate class to deal with future InputTransformers

github-actions · 2024-02-01T19:16:11Z

LocalStack Community integration with Pro

2 files ±0 2 suites ±0 1h 29m 23s ⏱️ - 2m 33s
2 721 tests ±0 2 466 ✅ ±0 255 💤 ±0 0 ❌ ±0
2 723 runs ±0 2 466 ✅ ±0 257 💤 ±0 0 ❌ ±0

Results for commit 9ce2706. ± Comparison against base commit be903b2.

This pull request removes 49 and adds 49 tests. Note that renamed tests count towards both.

tests.aws.services.events.test_events.TestEvents ‑ test_create_rule_with_one_unit_in_singular_should_succeed[rate(1 day)]
tests.aws.services.events.test_events.TestEvents ‑ test_create_rule_with_one_unit_in_singular_should_succeed[rate(1 hour)]
tests.aws.services.events.test_events.TestEvents ‑ test_create_rule_with_one_unit_in_singular_should_succeed[rate(1 minute)]
tests.aws.services.events.test_events.TestEvents ‑ test_put_event_with_content_base_rule_in_pattern
tests.aws.services.events.test_events.TestEvents ‑ test_put_events_into_event_bus[domain]
tests.aws.services.events.test_events.TestEvents ‑ test_put_events_into_event_bus[path]
tests.aws.services.events.test_events.TestEvents ‑ test_put_events_into_event_bus[standard]
tests.aws.services.events.test_events.TestEvents ‑ test_put_events_nonexistent_event_bus
tests.aws.services.events.test_events.TestEvents ‑ test_put_events_to_default_eventbus_for_custom_eventbus
tests.aws.services.events.test_events.TestEvents ‑ test_put_events_with_input_path
…

tests.aws.services.events.test_events.TestEvents ‑ test_event_pattern
tests.aws.services.events.test_events.TestEventsEventBus ‑ test_put_events_into_event_bus[domain]
tests.aws.services.events.test_events.TestEventsEventBus ‑ test_put_events_into_event_bus[path]
tests.aws.services.events.test_events.TestEventsEventBus ‑ test_put_events_into_event_bus[standard]
tests.aws.services.events.test_events.TestEventsEventBus ‑ test_put_events_nonexistent_event_bus
tests.aws.services.events.test_events.TestEventsEventBus ‑ test_put_events_to_default_eventbus_for_custom_eventbus
tests.aws.services.events.test_events_inputs.TestEventsInputPath ‑ test_put_events_with_input_path
tests.aws.services.events.test_events_inputs.TestEventsInputPath ‑ test_put_events_with_input_path_multiple
tests.aws.services.events.test_events_inputs.TestEventsInputTransformers ‑ test_put_events_with_input_transformation_to_sqs
tests.aws.services.events.test_events_integrations ‑ test_put_events_with_target_firehose
…

♻️ This comment has been updated with latest results.

coveralls · 2024-02-08T18:04:48Z

coverage: 83.861% (-0.02%) from 83.877%
when pulling 80d1a0c on refactor/events/clean-up-tests
into e2faf69 on master.

use class since InputTransformer tests will also be part of input test file

…ixtures

joe4dev

Good idea 💡

Refactoring tests can by definition not change any behavior, therefore this PR should have the semver label patch. The label feature also does not apply here.

Given that re-grouping is the main contribution of this PR, it would make sense to write a short docstring description at the top of the new test files to clarify the separation. Examples from Lambda:

localstack/tests/aws/services/lambda_/test_lambda_api.py

Line 1 in 1933c81

"""API-focused tests only.
localstack/tests/aws/services/lambda_/test_lambda_common.py

Line 1 in 1933c81

"""Testing different runtimes focusing on common functionality that should work across all runtimes (e.g., echo invoke).
localstack/tests/aws/services/lambda_/test_lambda_runtimes.py

Line 1 in 1933c81

"""Testing different Lambda runtimes focusing on specifics of particular runtimes (e.g., Nodejs ES6 modules).

So many unvalidated tests 😅 We have a mission 📈

joe4dev · 2024-03-20T17:35:32Z

tests/aws/services/events/conftest.py

+
+
+def sqs_collect_messages(
+    aws_client,


Did you remove the typings "SQSClient" deliberately?

Yes, I moved it to use the aws_client directly to keep it in line with how we generally call boto clients.

joe4dev · 2024-03-20T17:36:27Z

tests/aws/services/events/test_events.py

    @markers.aws.unknown
-    def test_put_events_with_values_in_array(self, aws_client, put_events_with_filter_to_sqs):
+    def test_put_events_with_values_in_array(sef, put_events_with_filter_to_sqs):


typo: self

fixed in: bf0255f

tests/aws/services/events/test_events.py

joe4dev · 2024-03-20T17:51:49Z

tests/aws/services/events/test_events.snapshot.json

@@ -1,6 +1,87 @@
 {


This snapshot file still contains some tests moved into other files. For example: tests/aws/services/events/test_events.py::TestEvents::test_put_rule. These should be removed. We don't have good tooling for maintaining snapshots during refactorings.

For such refactorings: I typically just delete the snapshot and validations files and re-create all snapshots from scratch.

joe4dev · 2024-03-20T18:19:29Z

tests/aws/services/events/test_events_integrations.py

+    assert put_response["FailedEntryCount"] == 0
+    assert put_response["FailedEntries"] == []
+
+    def check_stream_status():


Just an idea for later: Such waiters seem quite common in our test suite. We could consider adding a re-usable helper or even waiter (Dominik demoed how the boto waiters can be extended).

joe4dev · 2024-03-20T18:29:06Z

tests/aws/services/events/test_events.py

-
-        message_body = json.loads(messages[0]["Body"])
-        assert message_body["time"] == "2022-01-01T00:00:00Z"
-

 class TestEventsInputTransformers:


A good example to think where it belongs to:

Would input transformers fit into test_events_inputs?

Different integrations have different transformer behavior?

Or here?

fixed in: 8fa25b3

maxhoheiser force-pushed the refactor/events/clean-up-tests branch 4 times, most recently from 93ff37c to bfaf207 Compare February 7, 2024 09:13

feat: move put_events_with_filter_to_sqs fixture to conftest

0828f34

maxhoheiser force-pushed the refactor/events/clean-up-tests branch 3 times, most recently from e4d061d to d5bdfb2 Compare February 8, 2024 17:09

maxhoheiser self-assigned this Feb 8, 2024

maxhoheiser added the aws:events Amazon EventBridge label Feb 8, 2024

maxhoheiser force-pushed the refactor/events/clean-up-tests branch 2 times, most recently from e823011 to d802fb7 Compare February 8, 2024 17:18

maxhoheiser added type: feature New feature, or improvement to an existing feature semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases labels Feb 8, 2024

maxhoheiser force-pushed the refactor/events/clean-up-tests branch from 311039b to 56d2c0a Compare February 8, 2024 20:37

feat: extract sqs collect messages as fixture

7ad5476

maxhoheiser force-pushed the refactor/events/clean-up-tests branch from 56d2c0a to 7ad5476 Compare February 8, 2024 20:52

maxhoheiser added 5 commits February 8, 2024 21:58

feat: extract rule related tests into separate file

62a26fd

feat: extract tests for integration with other services

7619912

feat: extract inputPath tests

e309df3

use class since InputTransformer tests will also be part of input test file

feat: separate event bus tests into distinct class

93cacc8

feat: regenerate test event snapshots and validation dates

80d1a0c

maxhoheiser added this to the 3.2 milestone Feb 19, 2024

maxhoheiser modified the milestones: 3.2, 3.3 Feb 27, 2024

maxhoheiser added 2 commits March 19, 2024 17:00

Merge branch 'master' into refactor/events/clean-up-tests

1952f31

feat: align with incoming changes using functions instead of pytest f…

467f848

…ixtures

maxhoheiser requested a review from Morijarti March 19, 2024 16:49

maxhoheiser requested a review from joe4dev March 19, 2024 16:49

maxhoheiser marked this pull request as ready for review March 19, 2024 18:11

maxhoheiser requested review from dominikschubert and dfangl as code owners March 19, 2024 18:11

joe4dev approved these changes Mar 20, 2024

View reviewed changes

maxhoheiser added semver: patch Non-breaking changes which can be included in patch releases and removed type: feature New feature, or improvement to an existing feature labels Mar 25, 2024

maxhoheiser added 3 commits March 26, 2024 08:58

fix: typo

bf0255f

feat: move input transformer tests to inputs test file

8fa25b3

feat: add description to test files

81ac5ca

maxhoheiser removed the semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases label Mar 26, 2024

maxhoheiser added 2 commits March 26, 2024 17:10

feat: update snapshots

33f79e5

fix: increase retries dealing with aws delay

b13ca25

maxhoheiser force-pushed the refactor/events/clean-up-tests branch from af485bf to b13ca25 Compare March 26, 2024 16:10

fix: import functioned that was previously defined as fixture

9ce2706

maxhoheiser merged commit 26d849f into master Mar 26, 2024
29 checks passed

maxhoheiser deleted the refactor/events/clean-up-tests branch March 26, 2024 17:29

maxhoheiser mentioned this pull request Mar 26, 2024

Fix: Events: Refactor tests fix string formatting errors #10544

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor: Events: Clean up tests #10160

Refactor: Events: Clean up tests #10160

maxhoheiser commented Feb 1, 2024 •

edited

github-actions bot commented Feb 1, 2024 •

edited

coveralls commented Feb 8, 2024 •

edited

joe4dev left a comment

joe4dev Mar 20, 2024

maxhoheiser Mar 26, 2024

joe4dev Mar 20, 2024

maxhoheiser Mar 26, 2024

joe4dev Mar 20, 2024

joe4dev Mar 20, 2024

joe4dev Mar 20, 2024

maxhoheiser Mar 26, 2024

Refactor: Events: Clean up tests #10160

Refactor: Events: Clean up tests #10160

Conversation

maxhoheiser commented Feb 1, 2024 • edited

Motivation

Change

github-actions bot commented Feb 1, 2024 • edited

LocalStack Community integration with Pro

coveralls commented Feb 8, 2024 • edited

joe4dev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxhoheiser commented Feb 1, 2024 •

edited

github-actions bot commented Feb 1, 2024 •

edited

coveralls commented Feb 8, 2024 •

edited