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

[test-only] new lock tests #7595

Merged
merged 3 commits into from
Oct 27, 2023
Merged

[test-only] new lock tests #7595

merged 3 commits into from
Oct 27, 2023

Conversation

ScharfViktor
Copy link
Contributor

@ScharfViktor ScharfViktor commented Oct 26, 2023 •
edited
Loading

related #7561

We have lock test which we moved from core repo. Out of 98 scenarios related to locking, only 6 of them work.
and the existing tests can't:

  • set a timeout
  • work in project space
  • use space dav
  • no tests that check if the lock expires

I created first PR where:

  • user can create lock using different path
  • user can create lock using fileId - Collabora and onlyOfiice locks file using this way
  • user can set timeout
  • space members with edit permissions can create file lock
  • share receiver with edit permissions can create file lock - works only using fileId

found issue:

next test, next PRs:

  • unlock file
  • check if the lock expires

@update-docs
Copy link

update-docs bot commented Oct 26, 2023

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@ScharfViktor ScharfViktor marked this pull request as ready for review October 26, 2023 20:38
saw-jan
saw-jan reviewed Oct 27, 2023
View reviewed changes
Copy link
Member

@saw-jan saw-jan left a comment

Choose a reason for hiding this comment

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

I see that lockFileInProjectSpace and lockFileUsingFileId almost identical except fullUrl. Maybe we can have single method for both actions

tests/acceptance/features/bootstrap/WebDavLockingContext.php Outdated Show resolved Hide resolved
tests/acceptance/features/bootstrap/WebDavLockingContext.php Outdated Show resolved Hide resolved
tests/acceptance/features/bootstrap/WebDavLockingContext.php Outdated Show resolved Hide resolved
tests/acceptance/features/bootstrap/WebDavLockingContext.php Outdated Show resolved Hide resolved
@ScharfViktor ScharfViktor requested a review from saw-jan October 27, 2023 09:48
ScharfViktor
ScharfViktor commented Oct 27, 2023
View reviewed changes
tests/acceptance/features/bootstrap/WebDavLockingContext.php
@@ -219,10 +159,10 @@ private function lockFileUsingFileId(
$responseXml = $this->featureContext->getResponseXml($response, __METHOD__);
$xmlPart = $responseXml->xpath("//d:locktoken/d:href");
if (isset($xmlPart[0])) {
$this->tokenOfLastLock[$user][$filePath] = (string) $xmlPart[0];
$this->tokenOfLastLock[$user] = (string) $xmlPart[0];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@saw-jan I'll solve storing lock token when I combine lockFileInSpace() and lockFile() and move tests from coreApiWebdavLocks to apiSpaces

@ScharfViktor ScharfViktor requested a review from saw-jan October 27, 2023 11:44
saw-jan
saw-jan approved these changes Oct 27, 2023
View reviewed changes
Copy link
Member

@saw-jan saw-jan left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@ScharfViktor ScharfViktor merged commit 711c86e into master Oct 27, 2023
1 check passed
@delete-merged-branch delete-merged-branch bot deleted the createLockFileTest branch October 27, 2023 12:45
ownclouders pushed a commit that referenced this pull request Oct 27, 2023
@ScharfViktor
[test-only] new lock tests (#7595)
1d0a9e4 
* add new lock tests

* fix after review

* Update WebDavLockingContext.php
This was referenced Oct 30, 2023
Merged
Closed
S-Panta pushed a commit that referenced this pull request Oct 31, 2023
@ScharfViktor @S-Panta
[test-only] new lock tests (#7595)
4577e85 
* add new lock tests

* fix after review

* Update WebDavLockingContext.php
@S-Panta S-Panta mentioned this pull request Oct 31, 2023
Merged
S-Panta pushed a commit that referenced this pull request Oct 31, 2023
@ScharfViktor @S-Panta
[test-only] new lock tests (#7595)
bc70f2a 
* add new lock tests

* fix after review

* Update WebDavLockingContext.php
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@saw-jan saw-jan saw-jan approved these changes

@PrajwolAmatya PrajwolAmatya Awaiting requested review from PrajwolAmatya

@SagarGi SagarGi Awaiting requested review from SagarGi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ScharfViktor @saw-jan