[tests-only] test webdav previews by individual-it · Pull Request #37166 · owncloud/core

individual-it · 2020-03-25T12:24:14Z

Description

WebDAV API test for testing the previews of different types of files like text, images, pdf, etc. with valid as well as invalid preview size.

Related Issue

#37156

Motivation and Context

How Has This Been Tested?

locally

Screenshots (if appropriate):

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:

phil-davis · 2020-03-27T09:50:52Z

@individual-it @swoichha
PR #37173 has been merge to master. That should have fixed this issue. You will need to rebase this PR and then write the test scenarios so that they pass (with current master you should get "nice" 400 responses)

Remember that the scenarios will fail on older ownCloud. So skipOnOcV10.3 and skipOnOcV10.4.0

codecov · 2020-03-30T10:27:56Z

Codecov Report

Merging #37166 into master will not change coverage by %.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #37166   +/-   ##
=========================================
  Coverage     64.92%   64.92%           
  Complexity    19148    19148           
=========================================
  Files          1267     1267           
  Lines         74895    74895           
  Branches       1331     1331           
=========================================
  Hits          48626    48626           
  Misses        25877    25877           
  Partials        392      392

Flag	Coverage Δ	Complexity Δ
#javascript	`54.14% <ø> (ø)`	`0.00 <ø> (ø)`
#phpunit	`66.12% <ø> (ø)`	`19148.00 <ø> (ø)`

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 8ff9f92...0cc0426. Read the comment docs.

individual-it · 2020-04-02T07:58:34Z

 		$path = \str_replace($urlSpecialChar[0], $urlSpecialChar[1], $path);
+
+		if (!empty($urlParameter)) {
+			$urlParameter = http_build_query($urlParameter, '', '&');


Suggested change

$urlParameter = http_build_query($urlParameter, '', '&');

$urlParameter = \http_build_query($urlParameter, '', '&');

individual-it · 2020-04-02T08:01:02Z

+  Scenario: download previews of other users files
+    Given user "user1" has been created with default attributes and without skeleton files
+    And user "user0" has uploaded file "filesForUpload/lorem.txt" to "/parent.txt"
+    When user "user1" downloads the preview of "/parent.txt" with width "32" and height "32" using the WebDAV API


this step only downloads own previews not other users previews. We need an other step. something like

Suggested change

When user "user1" downloads the preview of "/parent.txt" with width "32" and height "32" using the WebDAV API

When user "user1" downloads the preview of "/parent.txt" of user "user0" with width "32" and height "32" using the WebDAV API

individual-it · 2020-04-02T08:04:00Z

+    Then the HTTP status code should be "404"
+    And the value of the item "/d:error/s:exception" in the response should be "Sabre\DAV\Exception\NotFound"
+
+  Scenario: set maximum size of previews


this scenario should set preview_max_x & preview_max_y and see what happens when a bigger preview is requested
see https://doc.owncloud.com/server/admin_manual/configuration/files/previews_configuration.html#maximum-preview-size

jasson99 · 2020-04-03T10:46:50Z

@@ -0,0 +1,129 @@
+@api @TestAlsoOnExternalUserBackend
+Feature: previews of files downloaded thought the webdav API


Suggested change

Feature: previews of files downloaded thought the webdav API

Feature: previews of files downloaded through the webdav API

dpakach · 2020-04-03T11:04:20Z

+	 * @param array $urlParameter
+	 * @param string $user2


Please write a line what these arguments are. Not really clear what it is.

Okay. I think $user2 does not need to be documented. If it should be then can you suggest something?

Please mention that the DAV request is sent to the dav-path of $user2's file

dpakach · 2020-04-06T03:46:09Z

+    Then the HTTP status code should be "200"
+    And the downloaded image should be "32" pixel wide and "32" pixel high
+
+  Scenario Outline: download previews of different file types


Suggested change

Scenario Outline: download previews of different file types

Scenario Outline: download previews of file types that doesn't support preview

dpakach · 2020-04-06T03:47:49Z

+    And the value of the item "/d:error/s:message" in the response should be "File with name parent.txt could not be located"
+    And the value of the item "/d:error/s:exception" in the response should be "Sabre\DAV\Exception\NotFound"
+
+  Scenario: disable previews


Suggested change

Scenario: disable previews

Scenario: Download file previews when it is disabled by the administrator

individual-it · 2020-04-06T06:49:36Z

-		$client = null
+		$client = null,
+		$urlParameter = [],
+		$user2 = null


Suggested change

$user2 = null

$doDavRequestAsUser = null

individual-it · 2020-04-06T06:50:21Z

 	 * @param int $timeout
 	 * @param Client|null $client
+	 * @param array $urlParameter to concatenate with path
+	 * @param string $user2


Suggested change

* @param string $user2

* @param string $doDavRequestAsUser run the DAV as this user, if null its same as $user

Talank

LGTM 👍

more reviews needed

individual-it · 2020-04-08T06:19:33Z

-		$password = null
+		$password = null,
+		$urlParameter = [],
+		$user2 = null


lets rename that variable like in the helper

individual-it · 2020-04-08T06:19:59Z

+	 *
+	 * @param $user1
+	 * @param $path
+	 * @param $user2


same here, lets have better variable names

individual-it

better variable names would make the code easier to understand

phil-davis

A few little things, and suggested refactoring to DRY code a bit.

phil-davis · 2020-04-08T10:33:37Z

+		$urlParameter = [
+			'x' => $width,
+			'y' => $height,
+			'forceIcon' => '0',
+			'preview' => '1'
+		];
+		$this->response = $this->makeDavRequest(
+			$user, "GET", $path, [], null, "files", 2, false, null, $urlParameter
+		);


Suggested change

$urlParameter = [

'x' => $width,

'y' => $height,

'forceIcon' => '0',

'preview' => '1'

];

$this->response = $this->makeDavRequest(

$user, "GET", $path, [], null, "files", 2, false, null, $urlParameter

);

$this->downloadPreviewOfOtherUser($user, $path, $user, $width, $height);

IMO this will work and avoid duplicating the $urlParameter variable setting.

phil-davis · 2020-04-08T10:37:46Z

+	}
+
+	/**
+	 * @Then the downloaded image should be :width pixel wide and :height pixel high


Suggested change

* @Then the downloaded image should be :width pixel wide and :height pixel high

* @Then the downloaded image should be :width pixels wide and :height pixels high

phil-davis · 2020-04-08T10:41:43Z

+    Then the HTTP status code should be "200"
+    And the downloaded image should be "32" pixel wide and "32" pixel high
+
+  Scenario Outline: download previews of file types that doesn't support preview


Suggested change

Scenario Outline: download previews of file types that doesn't support preview

Scenario Outline: download previews of file types that don't support preview

phil-davis · 2020-04-08T10:48:30Z

+    Then the HTTP status code should be "404"
+    And the value of the item "/d:error/s:exception" in the response should be "Sabre\DAV\Exception\NotFound"
+
+  Scenario: set maximum size of previews


Suggested change

Scenario: set maximum size of previews

Scenario: unset maximum size of previews

This scenario is actually testing that the maximum size of previews can be undefined.

Then, as @individual-it says, in another PR we can add test scenarios that set the max size to various values and check what happens when we ask for a bigger-size preview. Or you can add those now, since there are some changes to do anyway.

…views

…path

phil-davis

Let's get this merged, and then extra scenarios can be added in a later PR to test setting the maximum size of previews.

phil-davis · 2020-04-09T09:00:55Z

+    When the administrator updates system config key "preview_max_x" with value "null" using the occ command
+    And the administrator updates system config key "preview_max_y" with value "null" using the occ command


I thought that the test would be something like:

Suggested change

When the administrator updates system config key "preview_max_x" with value "null" using the occ command

And the administrator updates system config key "preview_max_y" with value "null" using the occ command

When the administrator updates system config key "preview_max_x" with value "32" using the occ command

And the administrator updates system config key "preview_max_y" with value "32" using the occ command

And then try to download a preview of size 64x64.

And see what happens.

phil-davis · 2020-04-09T09:08:20Z

Ooops - I meant to get this squashed, and forgot before merging. Too late now!

phil-davis · 2020-04-17T06:12:56Z

The new apiWebdavPreviews did not get added to .drone.star CI. So actually these tests are not running in CI. Fixed in PR #37274

individual-it mentioned this pull request Mar 25, 2020

requesting a file preview with an ivalid size results in HTTP error 500 #37165

Closed

phil-davis mentioned this pull request Mar 27, 2020

Apply proper error handling for unsupported file types and errors dur… #37173

Merged

11 tasks

individual-it assigned swoichha Mar 27, 2020

individual-it added 2 - Developing dev:acceptance-tests QA:team labels Mar 27, 2020

swoichha force-pushed the testWebDaVPreviews branch 3 times, most recently from 5a7e60e to 0b10f8a Compare March 31, 2020 05:33

individual-it commented Mar 31, 2020

View reviewed changes

Comment thread tests/acceptance/features/apiWebdavPreviews/previews.feature

swoichha force-pushed the testWebDaVPreviews branch 2 times, most recently from 12c67b9 to 978de97 Compare April 2, 2020 07:52

individual-it commented Apr 2, 2020

View reviewed changes

swoichha force-pushed the testWebDaVPreviews branch 2 times, most recently from 4c7eadb to bed149d Compare April 3, 2020 08:07

swoichha requested review from Talank, dpakach, haribhandari07, jasson99 and kiranparajuli589 April 3, 2020 08:09

jasson99 reviewed Apr 3, 2020

View reviewed changes

jasson99 marked this pull request as ready for review April 3, 2020 10:51

swoichha added 3 - To Review and removed 2 - Developing labels Apr 3, 2020

dpakach reviewed Apr 3, 2020

View reviewed changes

swoichha force-pushed the testWebDaVPreviews branch from bed149d to b863ba0 Compare April 3, 2020 11:47

dpakach reviewed Apr 6, 2020

View reviewed changes

individual-it commented Apr 6, 2020

View reviewed changes

Talank previously approved these changes Apr 8, 2020

View reviewed changes

individual-it commented Apr 8, 2020

View reviewed changes

swoichha force-pushed the testWebDaVPreviews branch from 6bbc098 to bd5a965 Compare April 8, 2020 06:45

individual-it commented Apr 8, 2020

View reviewed changes

Comment thread tests/acceptance/features/apiWebdavPreviews/previews.feature

swoichha requested a review from phil-davis April 8, 2020 09:10

phil-davis reviewed Apr 8, 2020

View reviewed changes

swoichha force-pushed the testWebDaVPreviews branch from a6ad4f8 to 0031158 Compare April 8, 2020 12:49

swoichha requested a review from phil-davis April 8, 2020 12:49

phil-davis reviewed Apr 9, 2020

View reviewed changes

Comment thread tests/acceptance/features/apiWebdavPreviews/previews.feature

swoichha force-pushed the testWebDaVPreviews branch from 0031158 to 2b98989 Compare April 9, 2020 03:29

individual-it and others added 8 commits April 9, 2020 12:49

test webdav previews

9abff5b

more tests

39c0011

added more tests

6f02979

added test scenarios for disable previews and set maximum size of pre…

3fec16a

…views

made changes in WebDav and WebDavHelper to pass special character in …

762c15b

…path

few scenario title changed

9afe681

added issue tag

d3c0999

added test to set/unset preview size and downloadPreviews method

0cc0426

swoichha force-pushed the testWebDaVPreviews branch from 2b98989 to 0cc0426 Compare April 9, 2020 07:05

swoichha requested a review from phil-davis April 9, 2020 08:22

phil-davis approved these changes Apr 9, 2020

View reviewed changes

phil-davis merged commit f78f3a8 into master Apr 9, 2020

delete-merged-branch Bot deleted the testWebDaVPreviews branch April 9, 2020 09:02

phil-davis mentioned this pull request Apr 9, 2020

test previews via webDAV API tests #37156

Closed

2 tasks

phil-davis mentioned this pull request Apr 11, 2020

[Tests-Only] skip new preview test scenarios on old ownCloud versions #37240

Merged

11 tasks

	$urlParameter = http_build_query($urlParameter, '', '&');
	$urlParameter = \http_build_query($urlParameter, '', '&');

	When user "user1" downloads the preview of "/parent.txt" with width "32" and height "32" using the WebDAV API
	When user "user1" downloads the preview of "/parent.txt" of user "user0" with width "32" and height "32" using the WebDAV API

		@@ -0,0 +1,129 @@
		@api @TestAlsoOnExternalUserBackend
		Feature: previews of files downloaded thought the webdav API

	Scenario Outline: download previews of different file types
	Scenario Outline: download previews of file types that doesn't support preview

	Scenario: disable previews
	Scenario: Download file previews when it is disabled by the administrator

	* @param string $user2
	* @param string $doDavRequestAsUser run the DAV as this user, if null its same as $user

	* @Then the downloaded image should be :width pixel wide and :height pixel high
	* @Then the downloaded image should be :width pixels wide and :height pixels high

	Scenario: set maximum size of previews
	Scenario: unset maximum size of previews

		When the administrator updates system config key "preview_max_x" with value "null" using the occ command
		And the administrator updates system config key "preview_max_y" with value "null" using the occ command

Uh oh!

Conversation

individual-it commented Mar 25, 2020 • edited by swoichha Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Uh oh!

phil-davis commented Mar 27, 2020

Uh oh!

codecov Bot commented Mar 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Talank left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

individual-it left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

phil-davis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

phil-davis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phil-davis commented Apr 9, 2020

Uh oh!

phil-davis commented Apr 17, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

individual-it commented Mar 25, 2020 •

edited by swoichha

Loading

codecov Bot commented Mar 30, 2020 •

edited

Loading