[SFN] [TestState] Add support for PRESENT and NONE field validation modes. - #13422
Conversation
Test Results - Preflight, Unit22 669 tests ±0 20 901 ✅ ±0 6m 20s ⏱️ -1s Results for commit 625b59a. ± Comparison against base commit 08d99fe2. ♻️ This comment has been updated with latest results. |
LocalStack Community integration with Pro 2 files ± 0 2 suites ±0 21m 34s ⏱️ +11s Results for commit 625b59a. ± Comparison against base commit 08d99fe2. ♻️ This comment has been updated with latest results. |
Test Results (amd64) - Acceptance7 tests ±0 5 ✅ ±0 3m 21s ⏱️ -1s Results for commit 625b59a. ± Comparison against base commit 08d99fe2. ♻️ This comment has been updated with latest results. |
Test Results (amd64) - Integration, Bootstrap 5 files ± 0 5 suites ±0 38m 53s ⏱️ -11s Results for commit 625b59a. ± Comparison against base commit 08d99fe2. ♻️ This comment has been updated with latest results. |
957a2e0 to
08d99fe
Compare
Also switches validation to use mock_input directly as TestStateMock is an overkill in this validation context.
c8a3600 to
625b59a
Compare
|
|
||
| @staticmethod | ||
| def validate_mock(mock: TestStateMock, definition: Definition, state_name: StateName) -> None: | ||
| def validate_mock(mock_input: MockInput, definition: Definition, state_name: StateName) -> None: |
There was a problem hiding this comment.
question: What happens if we don't define an errorOuput correctly? Pretty sure that needs to have the proper fields as well i.e Error and Cause. Could be worthwhile leaving the TestStateMock and doing that check here as well?
There was a problem hiding this comment.
I think it's worth separating 2 validations: one is result field validation against API shape, that is being done here, another is the rest of mock input. I guess the function name is misleading, I will rename it to a more specific validate_mock_result_input.
What you say about missing error output validation totally makes sense, I think we need to do in an upcoming PR, wdyt? And consolidate all validations that are not result shape ones in one place - now several of them are scattered around different methods. I also think it can be done on MockInput directly - TestStateMock at this stage is a bit of overkill at this stage, I tried it and it only causes creating more data access methods.
There was a problem hiding this comment.
Yeah let's address in a follow-up. I think seperating out the validations into result, errorOutput,stateConfiguration, and context.
Maybe we attach these validations as static methods to the TestStateMock?
48462aa
into
feature/step-functions/test-state-field-validation-mode-strict
Motivation
Add support for 2 remaining field validation modes.
Companion PR to #13419 .
Closes DRG-221.
Changes
PRESENTfield validation mode doesn't check the presence of required fields. It only validates the type and format of the fields that are present in the user-provided mockNONEmode means no format validation will be performed. A check that the result is a valid JSON string is still performed even inNONEfield validation mode.Also switches validation to use
mock_inputdirectly asTestStateMockis an overkill in this validation context.Tests
Related