-
Notifications
You must be signed in to change notification settings - Fork 32
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
feat(nextcloud_test): Add fixture recording and validation #1273
Conversation
cc0aced
to
864e969
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good from a quick glance
packages/dynamite/dynamite_runtime/lib/src/http_extensions.dart
Outdated
Show resolved
Hide resolved
Signed-off-by: jld3103 <[email protected]>
Signed-off-by: jld3103 <[email protected]>
864e969
to
c442291
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I tjink #1261 can also be marked to be closed.
Sure. Do you want me to create a dynamite_test package out of this so we can also utilize it for petstore and the e2e tests? |
I think that these packages can utilise nextxloud_test directly. I don't see the benefit of a separate package. |
The nextcloud_test package has a lot of nextcloud specific stuff so it's not very clean to add it to the dynamite packages which are not nextcloud specific. |
But it's only for testing 🤷♀️ |
Let's keep it as is for now but once we want this in other packages as well we should move it to a separate package. |
This system records HTTP requests and compares them to some saved pattern. This ensures we never have any changes in the raw requests that we don't know about.
Ideally we create a dynamite_test package so that we can reuse this system for dynamite_petstore_example and dynamite_end_to_end_test. I can do that in this PR already if it is favorable.
There is no performance impact to the tests:
#435