[tests-only][full-ci] add tests for checking the header location for tus upload #10807

nabim777 · 2024-12-24T10:39:58Z

Description

Since current test uses TUS library for tus operations and is unable to get and check the response.
Due to this, there was no check for the location header on TUS upload.
So, in this PR, following things are done:

TUS wrapper class is made and able to check the header response.
added step on tus scenario for status code check

Related Issue

[QA] test for zero-byte upload in the link share with secret file drop permissions #10649
Bug: Uploading a 0kb file to a secret file drop results in an error however the file appears in the folder multiple times. #10469

Motivation and Context

How Has This Been Tested?

test environment:
CI
locally
...

Screenshots (if appropriate):

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)
Technical debt
Tests only (no source changes)

Checklist:

Code changes
Unit tests added
Acceptance tests added
Documentation ticket raised:

tests/acceptance/features/apiSpacesShares/shareUploadTUS.feature

tests/acceptance/TestHelpers/TusClientWrapper.php

tests/acceptance/bootstrap/TUSContext.php

tests/acceptance/TestHelpers/TusClient.php

tests/acceptance/bootstrap/TUSContext.php

saw-jan · 2025-01-09T04:28:24Z

tests/acceptance/bootstrap/TUSContext.php

+	 * @throws GuzzleException
+	 * @throws ReflectionException
+	 */
+	public function publicUploadFileUsingTus(


can we use the above existing method (update if necessary)?

@saw-jan Could you check the changes I made to this function and let me know if they are what you meant?

Signed-off-by: nabim777 <[email protected]>

saw-jan · 2025-01-22T10:19:24Z

tests/acceptance/TestHelpers/TusClient.php

+	 * @return ResponseInterface
+	 * @throws GuzzleException
+	 */
+	public function createUploadWithResponse(string $key, int $bytes = -1): ResponseInterface {


since we want to reflect the existing methods but return differently, I recommend appending a char like this.

Suggested change

public function createUploadWithResponse(string $key, int $bytes = -1): ResponseInterface {

public function createWithUploadR(string $key, int $bytes = -1): ResponseInterface {

saw-jan · 2025-01-22T10:19:36Z

tests/acceptance/TestHelpers/TusClient.php

+	 * @throws GuzzleException
+	 * @throws TusException | ConnectionException
+	 */
+	public function uploadWithResponse(int $bytes = -1): ResponseInterface {


Suggested change

public function uploadWithResponse(int $bytes = -1): ResponseInterface {

public function uploadR(int $bytes = -1): ResponseInterface {

tests/acceptance/TestHelpers/TusClient.php

saw-jan · 2025-01-22T10:25:08Z

tests/acceptance/TestHelpers/TusClient.php

+		try {
+			$response = $this->getClient()->patch(
+				$this->getUrl(),
+				[
+				'body' => $data,
+				'headers' => $headers,
+				]
+			);
+		} catch (ClientException $e) {
+			throw $this->handleClientException($e);
+		}
+		return $response;


just return a response?

tests/acceptance/TestHelpers/TusClient.php

saw-jan · 2025-01-22T10:26:25Z

tests/acceptance/TestHelpers/TusClient.php

+		if ($this->isPartial()) {
+			$headers += ['Upload-Concat' => 'partial'];
+		}
+		try {


check if we need try catch

https://docs.guzzlephp.org/en/stable/request-options.html#http-errors

@saw-jan
First of all thanks for the docs.
As documet you have shared
can I add 'http_errors' => false, on post ?
it says it'll disable throwing exceptions on an HTTP protocol errors (i.e., 4xx and 5xx responses).

$response = $this->getClient()->post( $this->apiPath, [ 'body' => $data, 'headers' => $headers, 'http_errors' => false, ] );

amrita-shrestha · 2025-01-23T10:19:25Z

tests/acceptance/TestHelpers/TusClient.php

@@ -46,7 +46,7 @@ class TusClient extends Client {
 	 * @return ResponseInterface
 	 * @throws GuzzleException
 	 */
-	public function createUploadWithResponse(string $key, int $bytes = -1): ResponseInterface {
+	public function createWithUploadR(string $key, int $bytes = -1): ResponseInterface {


Suggested change

public function createWithUploadR(string $key, int $bytes = -1): ResponseInterface {

public function createUploadWithResponse(string $key, int $bytes = -1): ResponseInterface {

createUploadWithResponse is not a good name here. create with response seems a bit odd.
we are mimicking createWithUpload method from parent class but this time it retruns response. So, either createWithUploadR or createWithUploadRR - return response

you can have a comment for the method

can i have comment like this?
creates a resource with a post request and returns response

Signed-off-by: nabim777 <[email protected]>

sonarqubecloud · 2025-01-23T11:36:47Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

nabim777 self-assigned this Dec 24, 2024

nabim777 added the QA:team label Dec 24, 2024

nabim777 force-pushed the tests/check-header-location-for-tus-upload branch from b79bb57 to e9d5edd Compare December 24, 2024 11:07

nabim777 mentioned this pull request Dec 24, 2024

[QA] test for zero-byte upload in the link share with secret file drop permissions #10649

Open

9 tasks

nabim777 force-pushed the tests/check-header-location-for-tus-upload branch 2 times, most recently from e61de04 to c78d8a1 Compare December 27, 2024 04:47

nabim777 marked this pull request as ready for review December 27, 2024 04:48

nabim777 marked this pull request as draft December 27, 2024 04:48

nabim777 mentioned this pull request Dec 30, 2024

[tests-only][full-ci] Add tests for zero-byte file upload on public share #10697

Merged

9 tasks

nabim777 force-pushed the tests/check-header-location-for-tus-upload branch from c78d8a1 to 0ec975d Compare January 2, 2025 07:00

nabim777 marked this pull request as ready for review January 2, 2025 08:29

nabim777 requested review from saw-jan, anon-pradip, prashant-gurung899, Salipa-Gurung, amrita-shrestha, S-Panta, PrajwolAmatya and nirajacharya2 January 2, 2025 08:29

PrajwolAmatya reviewed Jan 2, 2025

View reviewed changes

tests/acceptance/features/apiSpacesShares/shareUploadTUS.feature Show resolved Hide resolved

tests/acceptance/features/apiSpacesShares/shareUploadTUS.feature Show resolved Hide resolved

saw-jan requested changes Jan 3, 2025

View reviewed changes

nabim777 requested review from saw-jan and PrajwolAmatya January 3, 2025 04:16

nirajacharya2 reviewed Jan 3, 2025

View reviewed changes

tests/acceptance/bootstrap/TUSContext.php Outdated Show resolved Hide resolved

tests/acceptance/TestHelpers/TusClient.php Outdated Show resolved Hide resolved

nabim777 force-pushed the tests/check-header-location-for-tus-upload branch from ee5defd to 91ea534 Compare January 3, 2025 04:22

nabim777 requested a review from nirajacharya2 January 3, 2025 04:23

nabim777 force-pushed the tests/check-header-location-for-tus-upload branch from 91ea534 to 0842b80 Compare January 3, 2025 05:17

nabim777 requested a review from phil-davis January 3, 2025 06:21

nabim777 force-pushed the tests/check-header-location-for-tus-upload branch 2 times, most recently from d592bae to fa52f68 Compare January 3, 2025 08:54

nabim777 force-pushed the tests/check-header-location-for-tus-upload branch 2 times, most recently from db2f7b6 to c860b81 Compare January 7, 2025 10:56

saw-jan reviewed Jan 14, 2025

View reviewed changes

nabim777 requested a review from saw-jan January 14, 2025 09:45

nabim777 force-pushed the tests/check-header-location-for-tus-upload branch 7 times, most recently from 9afc1e7 to d715bbf Compare January 21, 2025 11:21

nabim777 added 7 commits January 21, 2025 17:06

build an wrapper class for TUSUpload

68ea49d

Signed-off-by: nabim777 <[email protected]>

check the location header

01feab0

Signed-off-by: nabim777 <[email protected]>

rebasing master

b2ceb87

Signed-off-by: nabim777 <[email protected]>

review addresses

ef7cf00

Signed-off-by: nabim777 <[email protected]>

adds step for checking the status code of tus upload

2500f3c

Signed-off-by: nabim777 <[email protected]>

adds status check on given steps of tus upload

23c908b

Signed-off-by: nabim777 <[email protected]>

review resolve

87b01e9

Signed-off-by: nabim777 <[email protected]>

nabim777 force-pushed the tests/check-header-location-for-tus-upload branch 3 times, most recently from c8883d7 to f12a814 Compare January 22, 2025 03:43

make one function

a2fd9b6

Signed-off-by: nabim777 <[email protected]>

nabim777 force-pushed the tests/check-header-location-for-tus-upload branch from f12a814 to a2fd9b6 Compare January 22, 2025 08:38

saw-jan reviewed Jan 22, 2025

View reviewed changes

amrita-shrestha reviewed Jan 23, 2025

View reviewed changes

nabim777 force-pushed the tests/check-header-location-for-tus-upload branch 2 times, most recently from 1a16898 to 4381533 Compare January 23, 2025 11:05

review resolve

11993f0

Signed-off-by: nabim777 <[email protected]>

nabim777 force-pushed the tests/check-header-location-for-tus-upload branch from 4381533 to 11993f0 Compare January 23, 2025 11:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[tests-only][full-ci] add tests for checking the header location for tus upload #10807

[tests-only][full-ci] add tests for checking the header location for tus upload #10807

nabim777 commented Dec 24, 2024 •

edited

Loading

saw-jan Jan 9, 2025

nabim777 Jan 22, 2025

saw-jan Jan 22, 2025

saw-jan Jan 22, 2025

saw-jan Jan 22, 2025

saw-jan Jan 22, 2025

saw-jan Jan 22, 2025

nabim777 Jan 23, 2025 •

edited

Loading

amrita-shrestha Jan 23, 2025

saw-jan Jan 23, 2025

saw-jan Jan 23, 2025

nabim777 Jan 23, 2025 •

edited

Loading

sonarqubecloud bot commented Jan 23, 2025

	public function createUploadWithResponse(string $key, int $bytes = -1): ResponseInterface {
	public function createWithUploadR(string $key, int $bytes = -1): ResponseInterface {

	public function uploadWithResponse(int $bytes = -1): ResponseInterface {
	public function uploadR(int $bytes = -1): ResponseInterface {

[tests-only][full-ci] add tests for checking the header location for tus upload #10807

Are you sure you want to change the base?

[tests-only][full-ci] add tests for checking the header location for tus upload #10807

Conversation

nabim777 commented Dec 24, 2024 • edited Loading

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nabim777 Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nabim777 Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

sonarqubecloud bot commented Jan 23, 2025

Quality Gate passed

nabim777 commented Dec 24, 2024 •

edited

Loading

nabim777 Jan 23, 2025 •

edited

Loading

nabim777 Jan 23, 2025 •

edited

Loading