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 more tests on the data of translated CloudEvents. #62

Merged
merged 2 commits into from
Sep 15, 2020

Conversation

eamonnmcmanus
Copy link
Member

These tests check what is in the data field of the CloudEvents that we
translate from legacy events. For now they are not all that useful, because that
field is basically just a copy of the JSON payload of the legacy event. But we
do rearrange things slightly for PubSub events, and we are likely to do so for
Firebase events too in the future. Furthermore, if we eventually have a library
of classes representing the various event payloads, then we can use that library
instead of working with the JSON directly. That is what
google-cloudevents-dotnet does, for example.

These tests check what is in the `data` field of the CloudEvents that we
translate from legacy events. For now they are not all that useful, because that
field is basically just a copy of the JSON payload of the legacy event. But we
do rearrange things slightly for PubSub events, and we are likely to do so for
Firebase events too in the future. Furthermore, if we eventually have a library
of classes representing the various event payloads, then we can use that library
instead of working with the JSON directly. That is what
google-cloudevents-dotnet does, for example.
@jskeet
Copy link
Member

jskeet commented Sep 11, 2020

Will look on Monday

}

@Test
public void firestoreComplexData() throws IOException {
Copy link
Member

Choose a reason for hiding this comment

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

This test in .NET was mostly because we have helpers to make Firestore easier to work with. You probably don't need this test here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, the comment on lines 93–98 says we're not testing much here, but I think it is useful to show that we can successfully dissect a complex value, even if it is currently quite tedious and manual.

@eamonnmcmanus eamonnmcmanus merged commit 883f7af into master Sep 15, 2020
@kenneth-rosario kenneth-rosario deleted the datatests branch September 25, 2023 19:03
@kenneth-rosario kenneth-rosario restored the datatests branch September 25, 2023 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants