feat(stepfunctions): Add support for variables as TestState parameter#13827
feat(stepfunctions): Add support for variables as TestState parameter#13827
Conversation
tiurin
added
aws:stepfunctions
LocalStack Community integration with Pro 2 files 2 suites 22m 33s ⏱️ Results for commit 06e17a7. ♻️ This comment has been updated with latest results. |
Test Results (amd64) - Integration, Bootstrap 5 files 5 suites 39m 45s ⏱️ Results for commit 06e17a7. ♻️ This comment has been updated with latest results. |
tiurin
left a comment
tiurin
left a comment
There was a problem hiding this comment.
Added self-review notes.
| def add_inspection_data(self, env: TestStateEnvironment): | ||
| state = self._wrapped | ||
|
|
||
| if state._is_language_query_jsonpath(): |
There was a problem hiding this comment.
New test uncovered that JSONPath-related inspection data were added to states that use JSONata.
| self.inspection_data = InspectionData() | ||
| variables = variable_store.to_dict() | ||
| if variables: | ||
| self.inspection_data["variables"] = to_json_str(variables) |
There was a problem hiding this comment.
This is the key change, the rest is passing variables data through the call stack.
| _variable_declarations_cache: VariableDeclarations | None | ||
|
|
||
| def __init__(self): | ||
| def __init__(self, variables: dict | None = None): |
There was a problem hiding this comment.
With test state, variables store can now be pre-populated rather than being the result of evaluation.
| assigned_variables[traced_declaration_identifier] = traced_declaration_value_json_str | ||
| return assigned_variables | ||
|
|
||
| def to_dict(self) -> dict[str, str]: |
There was a problem hiding this comment.
This new function is similar to get_assigned_variables right above, but doesn't add any additional formatting.
| { | ||
| "tests/aws/services/stepfunctions/v2/outputdecl/test_output.py::TestArgumentsBase::test_base_cases[BASE_EMPTY]": { | ||
| "recorded-date": "04-11-2024, 13:15:46", | ||
| "recorded-date": "19-02-2026, 14:47:34", |
There was a problem hiding this comment.
Revalidated output tests since initially it looked like output-related problem. Keeping revalidated snapshots, even if they are not strictly related to this PR.
| for key, value in variables.items(): | ||
| self.set(key, value) |
There was a problem hiding this comment.
Q: should we be doing a deep copy here?
There was a problem hiding this comment.
No, since variables is used only once to set up execution environment for the test state. If any variable is modified later in the variable store it won't matter.
| ) | ||
| self.inspection_data = InspectionData() | ||
| variables = variable_store.to_dict() | ||
| if variables: |
There was a problem hiding this comment.
If we explicitly set variables to an empty dict, would we expect it to come up in inspection data? Suppose I'm wondering if this should be a None check or an is_truthy check
There was a problem hiding this comment.
Empty dict doesn't come up in inspection data. Good point, thanks! 👍 I've added a validated test for this situation.
Overriding __len__ broke many tests, there might be implicit checks like `if variable_store:` that were breaking with the override, outside of test state functionality. Changing to a more explicit length check
tiurin
force-pushed
the
sfn/fix/test-state-jsonata-variables/drg-526
branch
from
8298647 to
06e17a7
Compare
tiurin
added
notes: needed
|
Currently, only patch changes are allowed on main. Your PR labels (aws:stepfunctions, semver: minor, docs: skip, notes: needed) indicate that it cannot be merged into the main at this time. |
3 participants
Motivation
Fixes #13215
variablesparameter of TestState API call was not processed. This PR adds support for it.Changes
Set variables from API call parameter to test state execution environment.
Tests
Related