[Tests-Only] Refactor createShareToShareFolder.feature to use Shares folder #37881

phil-davis · 2020-09-03T11:24:57Z

Description

This is a way to get sharing API test scenarios that will be valid in both oC10 and OCIS. OCIS always puts received shares into a folder called Shares for the share receiver. On OCIS, auto-accept shares is always disabled.

keep the existing oC10 sharing scenarios that deliver shares into the root / folder of the share receiver. Tag them notToImplementOnOCIS. That maintains the test coverage of the default sharing on oC10 and the scenarios will not get run on OCIS.
make new scenarios in new feature files *ToSharesFolder.feature - for example createShareToSharesFolder.feature. In those scenarios, set the folder for received shares to "Shares".
Given the administrator has set the default folder for received shares to "Shares"
And adjust the test steps and expectations so that the share receiver expects the shares in /Shares folder.
Put no specific tag on these test scenarios - they should pass on both oC10 and OCIS.
That tests a full range of oC10 behavior when the default folder for received shares is configured, and also tests sharing behavior in OCIS.
in the new scenarios, disable auto-accept shares. On OCIS this will do nothing (auto-accept shares is already disabled). On oC10 this will provide the same expected behavior as on OCIS.
Given auto-accept shares has been disabled
And adjust the scenarios so that they have steps to accept a share before the receiver tries to access it. e.g.

    When user "Brian" accepts the share "/textfile0.txt" offered by user "Alice" using the sharing API
    Then the OCS status code should be "<ocs_status_code>"
    And the HTTP status code should be "200"
    And the content of file "/Shares/textfile0.txt" for user "Brian" should be "ownCloud test text file 0"

So the pattern of these new feature files will have a Background that has at least these lines:

  Background:
    Given the administrator has set the default folder for received shares to "Shares"
    And auto-accept shares has been disabled

And such features should work the same way on oC10 and OCIS - so the scenarios can be run on both.

Put the new feature files into new suites so that each suite still has a reasonable run-time. e.g. in this PR the existing apiShareManagementBasic suite becomes apiShareToRootManagementBasic and apiShareToSharesManagementBasic

Related Issue

owncloud/ocis#518

How Has This Been Tested?

CI

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Database schema changes (next release will require increase of minor version instead of patch)
Breaking change (fix or feature that would cause existing functionality to change)
Technical debt
Tests only (no source changes)

Checklist:

codecov · 2020-09-03T13:10:40Z

Codecov Report

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

@@            Coverage Diff            @@
##             master   #37881   +/-   ##
=========================================
  Coverage     64.75%   64.75%           
  Complexity    19403    19403           
=========================================
  Files          1285     1285           
  Lines         75823    75823           
  Branches       1336     1336           
=========================================
  Hits          49096    49096           
  Misses        26333    26333           
  Partials        394      394

Flag	Coverage Δ	Complexity Δ
#javascript	`54.06% <ø> (ø)`	`0.00 <ø> (ø)`
#phpunit	`65.93% <ø> (ø)`	`19403.00 <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

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 afb415a...dcf6137. Read the comment docs.

individual-it · 2020-09-07T07:56:33Z

Given the administrator has set the default folder for received shares to "Shares" - we need to understand that this step will also not do anything on OCIS and if s.o. sets it to s.th. else than Shares the tests in ocic will fail.
Do we need to document that?

phil-davis · 2020-09-07T08:03:45Z

Given the administrator has set the default folder for received shares to "Shares" - we need to understand that this step will also not do anything on OCIS and if s.o. sets it to s.th. else than Shares the tests in ocic will fail.
Do we need to document that?

For a start, I could change the words to:

Given the default folder for received shares has been set to "Shares"

That is more the standard grammar that we use. Not mentioning administrator makes it a bit less like an "action" step.

We could also say:

Given the default folder for received shares is "Shares"

That would not imply that any action has to be done - on some system-under-test it might need a setting to be set/changed, on another system-under-test it might be "is" already by default.

phil-davis · 2020-09-10T10:12:32Z

tests/acceptance/features/bootstrap/Sharing.php

@@ -1923,11 +1943,11 @@ public function checkingLastShareIDIsIncluded() {
 	}

 	/**
-	 * @Then /^the last share_id should not be included in the response/
+	 * @Then /^the last share id should not be included in the response/


Note: I took the opportunity to change the step text slightly.

phil-davis · 2020-09-10T10:12:43Z

tests/acceptance/features/bootstrap/Sharing.php

@@ -1966,15 +1986,15 @@ public function theResponseShouldNotContainAnyShareIds() {
 	}

 	/**
-	 * @Then user :user should not see share_id of last share
+	 * @Then user :user should not see the share id of the last share


Note: I took the opportunity to change the step text slightly.

phil-davis · 2020-09-10T10:12:52Z

tests/acceptance/features/bootstrap/Sharing.php

@@ -2462,7 +2482,7 @@ public function userHasDeletedPublicLinkShareNamedUsingTheSharingApi(
 	}

 	/**
-	 * @When /^user "([^"]*)" (declines|accepts) the share "([^"]*)" offered by user "([^"]*)" using the sharing API$/
+	 * @When /^user "([^"]*)" (declines|accepts) share "([^"]*)" offered by user "([^"]*)" using the sharing API$/


Note: I took the opportunity to change the step text slightly.

phil-davis · 2020-09-10T10:13:00Z

tests/acceptance/features/bootstrap/Sharing.php

@@ -2504,7 +2524,7 @@ public function userReactsToShareOfferedBy($user, $action, $share, $offeredBy) {
 	}

 	/**
-	 * @Given /^user "([^"]*)" has (declined|accepted) the share "([^"]*)" offered by user "([^"]*)"$/
+	 * @Given /^user "([^"]*)" has (declined|accepted) share "([^"]*)" offered by user "([^"]*)"$/


Note: I took the opportunity to change the step text slightly.

phil-davis · 2020-09-10T10:16:09Z

tests/acceptance/features/bootstrap/WebDav.php

@@ -494,7 +494,7 @@ public function slowdownDavRequests($method, $seconds) {
 	public function destinationHeaderValue($user, $fileDestination) {
 		$fullUrl = $this->getBaseUrl() . '/' .
 			WebDavHelper::getDavPath($user, $this->getDavPathVersion());
-		return $fullUrl . '/' . \ltrim($fileDestination, '/');
+		return \rtrim($fullUrl, '/') . '/' . \ltrim($fileDestination, '/');


When developing this PR, I noticed that some requests had a double-slash here. If WebDavHelper::getDavPath already ends with a slash / then we do not want to end up with 2 slashes. (the server was responding OK, but it will be nicer to test the ordinary case when the request URL has just a single slash between each part)

phil-davis · 2020-09-10T10:17:09Z

tests/acceptance/features/bootstrap/WebDav.php

@@ -1373,7 +1373,7 @@ public function asFileOrFolderShouldExist(
 		);
 		Assert::assertTrue(
 			$this->isEtagValid(),
-			"$entry '$path' expected to exist but not found"
+			"$entry '$path' expected to exist for user $user but not found"


I enhanced this message. In scenario outlines with multiple users, it was not clear which user was missing the expected file or folder.

phil-davis added 2 - Developing dev:acceptance-tests QA:team labels Sep 3, 2020

phil-davis self-assigned this Sep 3, 2020

phil-davis force-pushed the refactor-tests-to-use-Shares-folder branch from ed0d145 to b753dbb Compare September 4, 2020 06:38

phil-davis requested review from individual-it and dpakach September 7, 2020 07:15

individual-it approved these changes Sep 7, 2020

View reviewed changes

phil-davis force-pushed the refactor-tests-to-use-Shares-folder branch 2 times, most recently from 40bd9e0 to 806b69d Compare September 9, 2020 05:34

phil-davis marked this pull request as ready for review September 10, 2020 10:08

phil-davis added 4 - To release and removed 2 - Developing labels Sep 10, 2020

phil-davis commented Sep 10, 2020

View reviewed changes

phil-davis added 5 commits September 10, 2020 17:25

Refactor createShareToShareFolder.feature to use Shares folder

4d0d8d4

Adjust createShareToSharesFolder to disable auto-accept shares

6601919

Split apiShareManagementBasic suite

0609367

Refactor and add test scenarios deleteShareFromShares

458b0df

add test scenarios excludeGroupFromReceivingSharesToSharesFolder

304d853

phil-davis force-pushed the refactor-tests-to-use-Shares-folder branch from dcf6137 to 304d853 Compare September 10, 2020 11:46

phil-davis merged commit 65ee49a into master Sep 10, 2020

delete-merged-branch bot deleted the refactor-tests-to-use-Shares-folder branch September 10, 2020 11:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Tests-Only] Refactor createShareToShareFolder.feature to use Shares folder #37881

[Tests-Only] Refactor createShareToShareFolder.feature to use Shares folder #37881

phil-davis commented Sep 3, 2020 •

edited

Loading

codecov bot commented Sep 3, 2020 •

edited

Loading

individual-it commented Sep 7, 2020

phil-davis commented Sep 7, 2020

phil-davis Sep 10, 2020

phil-davis Sep 10, 2020

phil-davis Sep 10, 2020

phil-davis Sep 10, 2020

phil-davis Sep 10, 2020

phil-davis Sep 10, 2020

[Tests-Only] Refactor createShareToShareFolder.feature to use Shares folder #37881

[Tests-Only] Refactor createShareToShareFolder.feature to use Shares folder #37881

Conversation

phil-davis commented Sep 3, 2020 • edited Loading

Description

Related Issue

How Has This Been Tested?

Types of changes

Checklist:

codecov bot commented Sep 3, 2020 • edited Loading

Codecov Report

individual-it commented Sep 7, 2020

phil-davis commented Sep 7, 2020

phil-davis Sep 10, 2020

Choose a reason for hiding this comment

phil-davis Sep 10, 2020

Choose a reason for hiding this comment

phil-davis Sep 10, 2020

Choose a reason for hiding this comment

phil-davis Sep 10, 2020

Choose a reason for hiding this comment

phil-davis Sep 10, 2020

Choose a reason for hiding this comment

phil-davis Sep 10, 2020

Choose a reason for hiding this comment

phil-davis commented Sep 3, 2020 •

edited

Loading

codecov bot commented Sep 3, 2020 •

edited

Loading