-
Notifications
You must be signed in to change notification settings - Fork 195
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
[tests-only][full-ci] tests: adding test coverage for thumbnail service #10763
Conversation
8c65dfe
to
d0d2152
Compare
tests/acceptance/features/coreApiWebdavPreviews/previews.feature
Outdated
Show resolved
Hide resolved
tests/acceptance/features/coreApiWebdavPreviews/previews.feature
Outdated
Show resolved
Hide resolved
tests/acceptance/features/coreApiWebdavPreviews/previews.feature
Outdated
Show resolved
Hide resolved
tests/acceptance/features/coreApiWebdavPreviews/previews.feature
Outdated
Show resolved
Hide resolved
d0d2152
to
db38d55
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
db38d55
to
b0702a9
Compare
tests/acceptance/features/coreApiWebdavPreviews/previews.feature
Outdated
Show resolved
Hide resolved
b0702a9
to
5f38b97
Compare
0ca57bf
to
5f02f38
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
When user "Alice" downloads the preview of "/lorem-big.txt" with width "36" and height "36" and processor thumbnail using the WebDAV API | ||
Then the HTTP status code should be "403" |
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.
what happens if we do not set the configs? 200?
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.
what happens if we do not set the configs? 200?
depends on the image. dose it exceed the default values for the configs?
config | defaults |
---|---|
THUMBNAILS_MAX_INPUT_WIDTH | 7680 |
THUMBNAILS_MAX_INPUT_HEIGHT | 7680 |
THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE | 50MB |
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.
Good. It is understandable for image file but for a text file, how does the thumbnail service react? if the text file is under max input size?
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.
its same as an image file. if the text file is under max input size a thumbnail is created
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.
then the scenarios look good 👍
b046446
to
1ef4a83
Compare
167a0db
to
a04bbdf
Compare
a04bbdf
to
6fd5630
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
[tests-only][full-ci] tests: adding test coverage for thumbnail service
Description
When the image exceeded the any value in env
the server would return status code
500
which shouldn't be the case as mentioned in #10589This PR adds test coverage for issue #10589. making sure the correct status code is returned.
Imp discussion : #10763 (comment)
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: