[tests-only] test: adjust tests related to issue 1667 #9308
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
phil-davis
force-pushed
the
adjust-tests-issue-1667
branch
2 times, most recently
from
e25db19 to
a7ed141
Compare
saw-jan
approved these changes
Jun 4, 2024
phil-davis
force-pushed
the
adjust-tests-issue-1667
branch
from
a7ed141 to
96afdc3
Compare
|
ownclouders
pushed a commit
that referenced
this pull request
Jun 4, 2024
[tests-only] test: adjust tests related to issue 1667
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The issue was about test scenarios that send various unusual combinations of paths in the URL that have multiple
/characters together - sending paths like/remote.php//dav/spacessomeSpaceId/textfile1.txtPreviously some of these were "crashing" the server and returning 5xx HTTP status codes. But that has been fixed "some time ago". The test scenarios had expectations for various 2xx and 4xx status codes, but the actual exact codes that are returned are a bit different from what was put in the tests. This PR adjusts the expected codes so that they match what really happens.
There are different codes like 200, 201, 204, 400, 404, 412 etc. because these kind of requests are, in a sense, all invalid. But it is also easy for a human to see what is intended by each request, and some server code paths are actually parsing and processing these requests fine, resulting in
2xxcodes and the file action being done. Others are rejected, quite rightly, by the server code - the "invalid" format of the path might mean that the server cannot find the resource (404), or rejects the request (412, precondition failed etc.). I think that it is OK that the server responds in slightly different ways to these "bad" things. A least it does not give a 500 response any more.So this PR makes the test scenarios pass and "document" the current behavior. This way, we will know if some server code change causes a change in any of this behavior - the test scenario(s) will start failing.
I refactored test step
theHTTPStatusCodeOfResponsesOnEachEndpointShouldBeso that it better reports what the expected and actual status codes were when there is a problem. That was helpful to me when changing the test scenasrio expectations, and should be helpful to future developers.Related Issue
How Has This Been Tested?
CI
Types of changes
Checklist: