Add test for SSM GetParameterHistory API #11841

kakakakakku · 2024-11-13T16:14:38Z

Hi! I regularly use LocalStack, and thank you for such an awesome product😀

Motivation

Recently, while using LocalStack’s SSM Parameter Store, I checked the API coverage and found that some APIs were not marked in the "Internal Test Suite". So I wanted to contribute by adding tests.

https://docs.localstack.cloud/references/coverage/coverage_ssm/

Changes

In this Pull Request, I’ve added a test for the SSM GetParameterHistory API to improve coverage.

Testing

The tests passed in my local environment.

$ make test TEST_PATH=tests/aws/services/ssm
(snip)
tests/aws/services/ssm/test_ssm.py::TestSSM::test_describe_parameters PASSED                                                                                                                                                                                                                                                                                                                   [  7%]
tests/aws/services/ssm/test_ssm.py::TestSSM::test_put_parameters PASSED                                                                                                                                                                                                                                                                                                                        [ 14%]
tests/aws/services/ssm/test_ssm.py::TestSSM::test_hierarchical_parameter[/<param>//b//c] PASSED                                                                                                                                                                                                                                                                                                [ 21%]
tests/aws/services/ssm/test_ssm.py::TestSSM::test_hierarchical_parameter[<param>/b/c] PASSED                                                                                                                                                                                                                                                                                                   [ 28%]
tests/aws/services/ssm/test_ssm.py::TestSSM::test_get_secret_parameter PASSED                                                                                                                                                                                                                                                                                                                  [ 35%]
tests/aws/services/ssm/test_ssm.py::TestSSM::test_get_inexistent_secret PASSED                                                                                                                                                                                                                                                                                                                 [ 42%]
tests/aws/services/ssm/test_ssm.py::TestSSM::test_get_parameters_and_secrets PASSED                                                                                                                                                                                                                                                                                                            [ 50%]
tests/aws/services/ssm/test_ssm.py::TestSSM::test_get_parameters_by_path_and_filter_by_labels PASSED                                                                                                                                                                                                                                                                                           [ 57%]
tests/aws/services/ssm/test_ssm.py::TestSSM::test_get_parameter_by_arn PASSED                                                                                                                                                                                                                                                                                                                  [ 64%]
tests/aws/services/ssm/test_ssm.py::TestSSM::test_get_parameter_history PASSED                                                                                                                                                                                                                                                                                                                 [ 71%]
tests/aws/services/ssm/test_ssm.py::TestSSM::test_get_inexistent_maintenance_window PASSED                                                                                                                                                                                                                                                                                                     [ 78%]
tests/aws/services/ssm/test_ssm.py::TestSSM::test_trigger_event_on_systems_manager_change[standard] PASSED                                                                                                                                                                                                                                                                                     [ 85%]
tests/aws/services/ssm/test_ssm.py::TestSSM::test_trigger_event_on_systems_manager_change[domain] PASSED                                                                                                                                                                                                                                                                                       [ 92%]
tests/aws/services/ssm/test_ssm.py::TestSSM::test_trigger_event_on_systems_manager_change[path] PASSED                                                                                                                                                                                                                                                                                         [100%]

Could you please review? Thanks a lot 👍

localstack-bot

Welcome to LocalStack! Thanks for raising your first Pull Request and landing in your contributions. Our team will reach out with any reviews or feedbacks that we have shortly. We recommend joining our Slack Community and share your PR on the #community channel to share your contributions with us. Please make sure you are following our contributing guidelines and our Code of Conduct.

localstack-bot · 2024-11-13T16:14:56Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

kakakakakku · 2024-11-13T16:18:23Z

localstack-core/localstack/testing/pytest/fixtures.py

+        try:
+            aws_client.ssm.delete_parameter(Name=param)
+        except Exception as e:
+            LOG.debug("error cleaning up parameter %s: %s", param, e)


When I used the existing create_parameter method, I got the following error. Because the test calling create_parameter three times with the Overwrite=True parameter.

ERROR tests/aws/services/ssm/test_ssm.py::TestSSM::test_get_parameter_history - botocore.errorfactory.ParameterNotFound: An error occurred (ParameterNotFound) when calling the DeleteParameter operation: Parameter param-36e2b5a6 not found.

Since the parameter only needs to be deleted once, I added exception handling, referencing the existing implementation😀

https://github.com/localstack/localstack/blob/master/localstack-core/localstack/testing/pytest/fixtures.py#L163-L164

kakakakakku · 2024-11-13T16:19:16Z

I have read the CLA Document and I hereby sign the CLA

dominikschubert

Thanks for your contribution @kakakakakku !

Sorry for the delay, but I'd love to help you get this PR over the line. Let me know if you have questions regarding the suggest change.

dominikschubert · 2025-01-28T10:56:30Z

tests/aws/services/ssm/test_ssm.py

+        assert len(response["Parameters"]) == 3
+        assert response["Parameters"][0]["Value"] == "test"
+        assert response["Parameters"][0]["Version"] == 1
+        assert response["Parameters"][2]["Value"] == "test3"
+        assert response["Parameters"][2]["Version"] == 3


Typically we'd use our snapshot library in these cases, which will also catch changes to the API if we re-validate the case in the future. You can find more information on this in our contribution guidelines.

Please let me know if you'd like to tackle this yourself, otherwise I can also generate this for you! 👍

Thanks! I will add snapshot test soon :)

Question: When using snapshots test, are regular assertions like this unnecessary? Or is it better to have both? Looking at the existing tests in the same file, it seems there are tests using assertions. If it’s not needed, I’ll remove it 😀

Answer: "It depends" 😄 Sometimes it's good to keep manual ssertions, for example when testing for a specific behavior. When explicitly testing for API request/response surface though, we prefer having these recorded via snapshots only as they are easier to maintain and adapt to API changes.

kakakakakku · 2025-01-28T12:57:34Z

tests/aws/services/ssm/test_ssm.py

@@ -159,6 +159,25 @@ def test_get_parameter_by_arn(self, create_parameter, aws_client, snapshot, clea
        snapshot.match("Parameter", parameter_by_arn)
        snapshot.add_transformer(snapshot.transform.key_value("Name"))

+    @markers.aws.validated
+    @markers.snapshot.skip_snapshot_verify(paths=["$..Tier", "$..Policies"])


I believe that LocalStack does not support the Tier and Policies response fields. Therefore, I used the skip_snapshot_verify marker. Is this correct?

https://github.com/localstack/localstack/tree/master/docs/testing/parity-testing#skipping-verification-of-snapshot-test

$ awslocal ssm put-parameter --name sandbox --type String --value value1 { "Version": 1 } $ awslocal ssm put-parameter --name sandbox --type String --value value2 --overwrite { "Version": 2 } $ awslocal ssm put-parameter --name sandbox --type String --value value3 --overwrite { "Version": 3 } $ awslocal ssm get-parameter --name sandbox { "Parameter": { "Name": "sandbox", "Type": "String", "Value": "value3", "Version": 3, "LastModifiedDate": "2025-01-28T21:51:59.532000+09:00", "ARN": "arn:aws:ssm:ap-northeast-1:000000000000:parameter/sandbox", "DataType": "text" } }

Yes, transformers are used to get the recorded snapshot to a state that it can be repeatedly executed against the remote target. On the other hand the skip_snapshot_verify marker is used specifically to skip comparison for any part of the recorded snapshot that matches one of the patterns in the list. 👍

kakakakakku · 2025-01-28T13:00:10Z

tests/aws/services/ssm/test_ssm.py

+        assert len(response["Parameters"]) == 3
+        assert response["Parameters"][0]["Value"] == "test"
+        assert response["Parameters"][0]["Version"] == 1
+        assert response["Parameters"][2]["Value"] == "test3"
+        assert response["Parameters"][2]["Version"] == 3


Question: When using snapshots test, are regular assertions like this unnecessary? Or is it better to have both? Looking at the existing tests in the same file, it seems there are tests using assertions. If it’s not needed, I’ll remove it 😀

kakakakakku · 2025-01-28T13:01:58Z

tests/aws/services/ssm/test_ssm.py

+
+        snapshot.match("get-parameter-history-response", response)
+        snapshot.add_transformer(snapshot.transform.key_value("Name"))
+        snapshot.add_transformer(snapshot.transform.key_value("LastModifiedUser"))


Thank you for your kind support. I captured the snapshot in my personal AWS account 👍

$ export SNAPSHOT_UPDATE=1 $ export TEST_TARGET=AWS_CLOUD $ make test TEST_PATH=tests/aws/services/ssm

kakakakakku · 2025-01-28T14:07:41Z

@dominikschubert
Thank you for your feedback 😀 I rebased onto the latest master branch as some time had passed since this pull request was created. And I added snapshot tests. If there are any other mistakes, I will fix them soon.

dominikschubert

Thanks @kakakakakku for your contribution! Having better test coverage here is a great step forward in making the SSM service more reliable.

Appreciate your comments and your clear communication around what you did and why. 🙌

dominikschubert · 2025-07-29T11:35:41Z

tests/aws/services/ssm/test_ssm.py

@@ -159,6 +159,25 @@ def test_get_parameter_by_arn(self, create_parameter, aws_client, snapshot, clea
        snapshot.match("Parameter", parameter_by_arn)
        snapshot.add_transformer(snapshot.transform.key_value("Name"))

+    @markers.aws.validated
+    @markers.snapshot.skip_snapshot_verify(paths=["$..Tier", "$..Policies"])


Yes, transformers are used to get the recorded snapshot to a state that it can be repeatedly executed against the remote target. On the other hand the skip_snapshot_verify marker is used specifically to skip comparison for any part of the recorded snapshot that matches one of the patterns in the list. 👍

dominikschubert · 2025-07-29T11:37:03Z

tests/aws/services/ssm/test_ssm.py

+        assert len(response["Parameters"]) == 3
+        assert response["Parameters"][0]["Value"] == "test"
+        assert response["Parameters"][0]["Version"] == 1
+        assert response["Parameters"][2]["Value"] == "test3"
+        assert response["Parameters"][2]["Version"] == 3


Answer: "It depends" 😄 Sometimes it's good to keep manual ssertions, for example when testing for a specific behavior. When explicitly testing for API request/response surface though, we prefer having these recorded via snapshots only as they are easier to maintain and adapt to API changes.

kakakakakku · 2025-07-29T22:50:07Z

Thank you for your review. I learned a lot.

However, quite some time has passed since I submitted this pull request, and I believe there is now a significant gap with the latest main branch. Therefore, I will close this pull request.

I hope to have another opportunity to contribute in the future😀

kakakakakku requested a review from dominikschubert as a code owner November 13, 2024 16:14

localstack-bot reviewed Nov 13, 2024

View reviewed changes

kakakakakku commented Nov 13, 2024

View reviewed changes

localstack-bot added a commit that referenced this pull request Nov 13, 2024

@kakakakakku has signed the CLA in #11841

f3ab9a5

kakakakakku mentioned this pull request Nov 16, 2024

Add test for SSM GetParameter API by parameter name #11858

Closed

kakakakakku mentioned this pull request Dec 3, 2024

Add test for EventBridge Scheduler TagResource API and UntagResource API #11976

Merged

silv-io added this to the Playground milestone Jan 24, 2025

silv-io added the semver: patch Non-breaking changes which can be included in patch releases label Jan 24, 2025

dominikschubert requested changes Jan 28, 2025

View reviewed changes

kakakakakku added 2 commits January 28, 2025 20:35

Add test for SSM GetParameterHistory API

4e3378a

Add test for SSM GetParameterHistory API (Snapshot)

0f0b995

kakakakakku force-pushed the add-tests-for-ssm-get-parameter-history branch from 77a29fb to 0f0b995 Compare January 28, 2025 12:53

kakakakakku commented Jan 28, 2025

View reviewed changes

alexrashed requested a review from dominikschubert July 25, 2025 07:37

dominikschubert approved these changes Jul 29, 2025

View reviewed changes

kakakakakku closed this Jul 29, 2025

Uh oh!

Add test for SSM GetParameterHistory API #11841

Add test for SSM GetParameterHistory API #11841

Uh oh!

Conversation

kakakakakku commented Nov 13, 2024

Motivation

Changes

Testing

Uh oh!

localstack-bot left a comment

Choose a reason for hiding this comment

Uh oh!

localstack-bot commented Nov 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kakakakakku commented Nov 13, 2024

Uh oh!

dominikschubert left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kakakakakku commented Jan 28, 2025

Uh oh!

dominikschubert left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kakakakakku commented Jul 29, 2025

Uh oh!

Uh oh!

localstack-bot commented Nov 13, 2024 •

edited

Loading