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

[Test-only] reva now returns 404 when trying to delete a nonexisting file #37149

Merged
merged 2 commits into from
Mar 20, 2020

Conversation

butonic
Copy link
Member

@butonic butonic commented Mar 20, 2020

@butonic butonic requested a review from individual-it March 20, 2020 13:06
@butonic butonic self-assigned this Mar 20, 2020
@butonic butonic changed the title reva now returns 404 when trying to deleta a nonexisting file reva now returns 404 when trying to delet a nonexisting file Mar 20, 2020
@butonic butonic changed the title reva now returns 404 when trying to delet a nonexisting file reva now returns 404 when trying to delete a nonexisting file Mar 20, 2020
@codecov
Copy link

codecov bot commented Mar 20, 2020

Codecov Report

Merging #37149 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #37149   +/-   ##
=========================================
  Coverage     64.85%   64.85%           
  Complexity    19136    19136           
=========================================
  Files          1267     1267           
  Lines         74895    74895           
  Branches       1331     1331           
=========================================
  Hits          48575    48575           
  Misses        25928    25928           
  Partials        392      392
Flag Coverage Δ Complexity Δ
#javascript 54.14% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 66.05% <ø> (ø) 19136 <ø> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da92e15...743cd81. Read the comment docs.

@mmattel
Copy link
Contributor

mmattel commented Mar 20, 2020

You may want to change the title to [Tests-Only] reva now... as it covers tests only. Keeps things better identifyable

@butonic butonic changed the title reva now returns 404 when trying to delete a nonexisting file [Test-only] reva now returns 404 when trying to delete a nonexisting file Mar 20, 2020
Comment on lines +1279 to +1283
if ($response->getStatusCode() < 401 || $response->getStatusCode() > 404) {
throw new \Exception(
"$entry '$path' expected to not exist " .
"(status code {$response->getStatusCode()}, expected 401 - 404)"
);
Copy link
Contributor

@phil-davis phil-davis Mar 20, 2020

Choose a reason for hiding this comment

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

We like to use Assert where we can in Then steps - it provides a nicer output on failure than a long traceback. But that sort of stuff has been refactored more recently, so I guess the revert has done this.
We can adjust that later, since it will be better to get this PR merged so that OCIS-related tests can pass.

@phil-davis phil-davis merged commit f9098d0 into master Mar 20, 2020
@delete-merged-branch delete-merged-branch bot deleted the ocis-tests-expect-40x-on-not-exist branch March 20, 2020 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants