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

deleting a file when 2 files exist with different case #33597

Merged
merged 3 commits into from
Nov 22, 2018

Conversation

davitol
Copy link
Contributor

@davitol davitol commented Nov 20, 2018

Related Issue

#32357 part

Motivation and Context

Spread Tests

How Has This Been Tested?

Local test runs

@codecov
Copy link

codecov bot commented Nov 21, 2018

Codecov Report

Merging #33597 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #33597      +/-   ##
============================================
+ Coverage     64.22%   64.22%   +<.01%     
+ Complexity    18431    18269     -162     
============================================
  Files          1193     1193              
  Lines         69076    69077       +1     
  Branches       1277     1277              
============================================
+ Hits          44363    44364       +1     
  Misses        24341    24341              
  Partials        372      372
Flag Coverage Δ Complexity Δ
#javascript 52.91% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 65.53% <ø> (ø) 18269 <ø> (-162) ⬇️
Impacted Files Coverage Δ Complexity Δ
apps/files/lib/Command/VerifyChecksums.php 96.42% <0%> (+0.04%) 37% <0%> (ø) ⬇️

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 266c5e6...3e4da84. Read the comment docs.

@davitol davitol force-pushed the add-test-deleting-file-different-case branch from 79424ff to 47b5c58 Compare November 21, 2018 10:42
Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

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

This scenario fails for me locally. I don't know how it passed CI on drone - maybe related to the recent drone problems???

I suggest that each file should have different content. Then delete one of the files. Check that the deleted file does not exist, check that the remaining file has the expected content. That will confirm that both the correct file name and the correct file content were deleted.

@phil-davis phil-davis dismissed individual-it’s stale review November 22, 2018 14:21

code has been adjusted

@phil-davis
Copy link
Contributor

I looked at the current drone logs and the new scenario is being run, and it passes.
https://drone.owncloud.com/owncloud/core/12889/406

  Scenario Outline: delete a file when 2 files exist with different case                   # /drone/src/tests/acceptance/features/apiWebdavOperations/deleteFile.feature:22
    Given using <dav_version> DAV path                                                     # FeatureContext::usingOldOrNewDavPath()
    And user "user0" has uploaded file with content "to delete" to "/textfile1.txt"        # FeatureContext::userUploadsAFileWithContentTo()
    And user "user0" has uploaded file with content "uploaded content" to "/TextFile1.txt" # FeatureContext::userUploadsAFileWithContentTo()
    When user "user0" deletes file "/textfile1.txt" using the WebDAV API                   # FeatureContext::userDeletesFile()
    Then the HTTP status code should be "204"                                              # FeatureContext::theHTTPStatusCodeShouldBe()
    And as "user0" file "/textfile1.txt" should not exist                                  # FeatureContext::asFileOrFolderShouldNotExist()
    And as "user0" file "/TextFile1.txt" should exist                                      # FeatureContext::asFileOrFolderShouldExist()
    And the content of file "/TextFile1.txt" for user "user0" should be "uploaded content" # FeatureContext::contentOfFileForUserShouldBe()

    Examples:
      | dav_version |
      | old         |
| new         |

So at least we now know that CI runs the new scenario.

@phil-davis phil-davis merged commit 0a19245 into master Nov 22, 2018
@phil-davis phil-davis deleted the add-test-deleting-file-different-case branch November 22, 2018 14:47
@phil-davis
Copy link
Contributor

@davitol when making the backport, please make sure that you have pulled the very latest stable10 of core, and branch from that. Maybe the problems we have been having are related to PRs branched from old master or stable10 - I don't know.

@davitol
Copy link
Contributor Author

davitol commented Nov 23, 2018

Backport in : #33633

@lock lock bot locked as resolved and limited conversation to collaborators Nov 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants