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.

Sign up for GitHub

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

Jump to bottom

More API test for file versions #31628

Merged
merged 1 commit into from
Jun 6, 2018
Merged

More API test for file versions #31628

merged 1 commit into from
Jun 6, 2018

Conversation

paurakhsharma
Copy link
Member

Description

This PR adds more tests for file versions which were missing in #31577

Related Issue

#31535

How Has This Been Tested?

Locally

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Acceptance test

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@paurakhsharma paurakhsharma added this to the development milestone Jun 4, 2018
@paurakhsharma paurakhsharma self-assigned this Jun 4, 2018
phil-davis
phil-davis reviewed Jun 5, 2018
View reviewed changes
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.

Few small comments, and 1 scenario failing.

tests/acceptance/features/apiMain/dav-versions.feature Outdated
And the downloaded content when downloading file "/sharingfolder/sharefile.txt" for user "user0" with range "bytes=0-12" should be "old content"

Scenario: sharer restores the file inside a shared folder created by sharee and modified by sharee
Copy link
Contributor

Choose a reason for hiding this comment

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

Description here is the same as line 116.
Maybe it should say:

sharee restores the file inside a shared folder created by sharer and modified by sharer

?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes I forgot to modify the description

tests/acceptance/features/apiMain/dav-versions.feature Outdated
And the downloaded content when downloading file "/sharingfolder/sharefile.txt" for user "user1" with range "bytes=0-12" should be "old content"

Scenario: sharer restores the file inside a shared folder modified by sharee
Copy link
Contributor

Choose a reason for hiding this comment

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

This scenario fails.

tests/acceptance/features/apiMain/dav-versions.feature Outdated
And user "user0" has uploaded file with content "user0 content" to "/sharingfolder/sharefile.txt"
And user "user1" has uploaded file with content "user1 content" to "/sharingfolder/sharefile.txt"
And user "user2" has uploaded file with content "user2 content" to "/sharingfolder/sharefile.txt"
Copy link
Contributor

Choose a reason for hiding this comment

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

user2 has no access - maybe you meant to share the folder with the group?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah my fault, I wanted to share with the group

@codecov
Copy link

codecov bot commented Jun 6, 2018 •
edited

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #31628   +/-   ##
=========================================
  Coverage     62.87%   62.87%           
  Complexity    18409    18409           
=========================================
  Files          1151     1151           
  Lines         69114    69114           
  Branches       1260     1260           
=========================================
  Hits          43458    43458           
  Misses        25287    25287           
  Partials        369      369
Flag Coverage Δ Complexity Δ
#javascript 52.39% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 64.08% <ø> (ø) 18409 <ø> (ø) ⬇️

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 27695e4...b554d3a. Read the comment docs.

@phil-davis
Copy link
Contributor

phil-davis commented Jun 6, 2018 •
edited

There was a "docker daemon" silly error - I restarted drone CI.

@phil-davis
Copy link
Contributor

@paurakhsharma somehow the drone run that passed has not reported the good result back to GitHub.
I will try a rebase.

@phil-davis
Copy link
Contributor

@ownclouders rebase

@phil-davis phil-davis force-pushed the API-more-tests-file_versions branch from dc3da3f to b554d3a Compare June 6, 2018 10:48
@phil-davis
Copy link
Contributor

ownclouders seem to be asleep. I rebased locally and pushed. Let's see what happens to the next CI run.
@paurakhsharma you can make the backport PR.

@ownclouders
Copy link
Contributor

Hey! I'm GitMate.io! This pull request is being rebased automatically. Please DO NOT push while rebase is in progress or your changes would be lost permanently ⚠️

@ownclouders
Copy link
Contributor

Automated rebase with GitMate.io was successful! 🎉

@phil-davis
Copy link
Contributor

Oh, he (or she?) woke up and rebased also. Let's see.

@paurakhsharma paurakhsharma merged commit f61383a into master Jun 6, 2018
@paurakhsharma
Copy link
Member Author

I am getting fatal: bad object xxxxxxx error while trying to cherry-pick this commit. What might be the case?

@phil-davis
Copy link
Contributor

This was rebased directly on GitHub. So you need to pull from GitHub so that you get the commit here into your local git repo.

@paurakhsharma paurakhsharma mentioned this pull request Jun 7, 2018
Merged
@paurakhsharma
Copy link
Member Author

Backport on #31675

@phil-davis phil-davis deleted the API-more-tests-file_versions branch June 7, 2018 09:15
@phil-davis phil-davis mentioned this pull request Jun 7, 2018
Merged
8 tasks
@PVince81 PVince81 modified the milestones: development, QA Jun 13, 2018
@lock
Copy link

lock bot commented Jul 31, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jul 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@phil-davis phil-davis phil-davis approved these changes

Assignees

@paurakhsharma paurakhsharma

Labels
3 - To Review dev:acceptance-tests
Projects
None yet
Milestone
10.0.9
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@paurakhsharma @phil-davis @ownclouders @PVince81