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.

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

Add formatting-aware timestamp replacement via new TimestampTransformer #9772

Merged
merged 12 commits into from Dec 11, 2023

Conversation

dominikschubert
Copy link
Member

@dominikschubert dominikschubert commented Nov 29, 2023

Motivation

In the past we've had issues with missing parity of timestamp formats due to the default transformer replacing all timestamps with the same generic tokens. This is an attempt to improve on this by encoding the date format. It's intentionally not trying to be too smart by doing too much magic(e.g. trying to parse & infer formatting) and needs to be explicitly extended by regex + replacement.

I really like how Golang does date formatting, so in that style, the replacements are using a reference timestamp: 2022-07-13T13:48:01Z which is the UTC timestamp of the v1.0.0 commit of localstack/localstack: cf26bd9

Changes

  • New TimestampTransformer
    • replacement with <datetime> for anything that boto deserializes as a datetime.datetime (typically epoch numbers in the API)
    • replacement with <timestamp:...> for serialized timestamps, e.g. "2022-07-13T13:48:01.000000+00:00" => "<timestamp:2022-07-13T13:48:01.000000+00:00>"
  • Added an exemption list that matches based on substrings of the executed test's file path. Anything that matches this list will keep using the old transformer, while anything else is now using the new TimestampTransformer by default.
    • This means that it's fairly easy to incrementally migrate/regenerate snapshots and avoids a big-bang merge with a lot of merge conflicts.

Examples

Example from a currently failing run:

=================================== FAILURES ===================================
_____________ TestS3PresignedUrl.test_s3_presigned_url_expired[s3] _____________
>> match key: expired-exception
	(~) /Error/Expires 'date' → '<timestamp:2022-07-13T13:48:01.000Z>' ... (expected → actual)
	(~) /Error/ServerTime 'date' → '<timestamp:2022-07-13T13:48:01.000Z>' ... (expected → actual)

Follow-Ups

What's left to do:

  • add follow-up PR that removes the exemptions (stays open until both community & -ext are migrated)
  • adjust snapshots in -ext
  • adjust snapshots in community

@dominikschubert dominikschubert self-assigned this Nov 29, 2023
@dominikschubert dominikschubert added the semver: patch Non-breaking changes which can be included in patch releases label Nov 29, 2023
@dominikschubert dominikschubert changed the title POC: Snapshot timestamp & datetime transformer Add formatting-aware timestamp replacement via new TimestampTransformer Dec 6, 2023
@coveralls
Copy link

Coverage Status

coverage: 84.248% (-0.03%) from 84.277%
when pulling edf9715 on snapshot-timestamp-transformer
into 01f68f8 on master.

Copy link

github-actions bot commented Dec 6, 2023

LocalStack Community integration with Pro

       2 files         2 suites   1h 11m 44s ⏱️
2 396 tests 2 168 ✔️ 228 💤 0
2 397 runs  2 168 ✔️ 229 💤 0

Results for commit edf9715.

@dominikschubert dominikschubert marked this pull request as ready for review December 6, 2023 20:18
@dominikschubert dominikschubert added this to the 3.1 milestone Dec 7, 2023
Copy link
Member

@steffyP steffyP left a comment

Choose a reason for hiding this comment

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

Great work 🚀 👍 Love that we can now check the actual expected date format 👏

the replacements are using a reference timestamp: 2022-07-13T13:48:01Z which is the UTC timestamp of the v1.0.0 commit of localstack/localstack

nice, I remember we once talked about this approach 😄

This means that it's fairly easy to incrementally migrate/regenerate snapshots and avoids a big-bang merge with a lot of merge conflicts.

👍 🙇‍♀️

localstack/testing/pytest/snapshot.py Show resolved Hide resolved
@dominikschubert dominikschubert merged commit 2b03dee into master Dec 11, 2023
28 checks passed
@dominikschubert dominikschubert deleted the snapshot-timestamp-transformer branch December 11, 2023 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants