Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed snapshot assert to field check #1911

Closed
wants to merge 1 commit into from
+12 −33
Closed

Changed snapshot assert to field check #1911

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 0 additions & 27 deletions tests_integration/nativeapp/__snapshots__/test_version.ambr
View file
Open in desktop

This file was deleted.

18 changes: 12 additions & 6 deletions tests_integration/nativeapp/test_version.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,12 +372,18 @@ def test_nativeapp_version_create_package_no_magic_comment(

# app package contains version v1 with 2 patches
actual = runner.invoke_with_connection_json(split(list_command))
for row in actual.json:
# Remove date field
row.pop("created_on", None)
# Remove metric_level field
row.pop("metric_level", None)
assert actual.json == snapshot
assert len(actual.json) == 2
_assert_list_of_dicts_contains_key_with_value(actual.json, "patch", 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is significantly weakening the assertion. Since the JSON output is part of the contract, I'd prefer checking that the actual dict is a superset of the expected one instead. Do you want to make the change or would you prefer that we do it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't know which fields should I include, treat this as a proposition. Would be great if you could do this 🙏 .

Copy link
Contributor

Choose a reason for hiding this comment

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

ok feel free to close this, I'll submit a PR soon.

_assert_list_of_dicts_contains_key_with_value(actual.json, "patch", 1)


def _assert_list_of_dicts_contains_key_with_value(
dicts: List[dict], key: str, value: Any
) -> None:
for d in dicts:
if key in d and d[key] == value:
return
raise AssertionError(f"No patch version '{value}' in app package.")


# Tests a simple flow of an existing project, executing snow app version create, drop and teardown, all with distribution=internal
Expand Down
Loading