[full-ci][tests-only] test: fix some test flakiness (#2003)

* test: check content after upload

Signed-off-by: Saw-jan <saw.jan.grg3e@gmail.com>

test: check content with retry

Signed-off-by: Saw-jan <saw.jan.grg3e@gmail.com>

* test: check empty body before json decoding

Signed-off-by: Saw-jan <saw.jan.grg3e@gmail.com>

* test: wait post-processing for webdav requests if applicable

Signed-off-by: Saw-jan <saw.jan.grg3e@gmail.com>

* test: check token before doing request

Signed-off-by: Saw-jan <saw.jan.grg3e@gmail.com>

test: check body before json decoding

Signed-off-by: Saw-jan <saw.jan.grg3e@gmail.com>

test: add wait step

Signed-off-by: Saw-jan <saw.jan.grg3e@gmail.com>

---------

Signed-off-by: Saw-jan <saw.jan.grg3e@gmail.com>

tests/acceptance/TestHelpers/HttpRequestHelper.php

@@ -33,6 +33,7 @@ use SimpleXMLElement;
 use Sabre\Xml\LibXMLException;
 use Sabre\Xml\Reader;
 use GuzzleHttp\Pool;
+use Symfony\Component\HttpFoundation\Response;
 
 /**
  * Helper for HTTP requests
@@ -74,7 +75,6 @@ class HttpRequestHelper {
 	 *                     than download it all up-front.
 	 * @param int|null $timeout
 	 * @param Client|null $client
-	 * @param string|null $bearerToken
 	 *
 	 * @return ResponseInterface
 	 * @throws GuzzleException
@@ -92,8 +92,42 @@ class HttpRequestHelper {
 		bool $stream = false,
 		?int $timeout = 0,
 		?Client $client =  null,
-		?string $bearerToken = null
 	): ResponseInterface {
+		$bearerToken = null;
+		if (TokenHelper::useBearerToken() && $user && $user !== 'public') {
+			$bearerToken = TokenHelper::getTokens($user, $password, $url)['access_token'];
+			// check token is still valid
+			$parsedUrl = parse_url($url);
+			$baseUrl = $parsedUrl['scheme'] . '://' . $parsedUrl['host'];
+			$baseUrl .= isset($parsedUrl['port']) ? ':' . $parsedUrl['port'] : '';
+			$testUrl = $baseUrl . "/graph/v1.0/use/$user";
+			if (OcHelper::isTestingOnReva()) {
+				$url = $baseUrl . "/ocs/v2.php/cloud/users/$user";
+			}
+			// check token validity with a GET request
+			$c = self::createClient(
+				$user,
+				$password,
+				$config,
+				$cookies,
+				$stream,
+				$timeout,
+				$bearerToken
+			);
+			$testReq = self::createRequest($testUrl, $xRequestId, 'GET');
+			try {
+				$testRes = $c->send($testReq);
+			} catch (RequestException $ex) {
+				$testRes = $ex->getResponse();
+				if ($testRes && $testRes->getStatusCode() === Response::HTTP_UNAUTHORIZED) {
+					// token is invalid or expired, get a new one
+					echo "[INFO] Bearer token expired or invalid, getting a new one...\n";
+					TokenHelper::clearAllTokens();
+					$bearerToken = TokenHelper::getTokens($user, $password, $url)['access_token'];
+				}
+			}
+		}
+
 		if ($client === null) {
 			$client = self::createClient(
 				$user,
@@ -160,6 +194,24 @@ class HttpRequestHelper {
 		}
 
 		HttpLogger::logResponse($response);
+
+		// wait for post-processing to finish if applicable
+		if (WebdavHelper::isDAVRequest($url)
+			&& \str_starts_with($url, OcHelper::getServerUrl())
+			&& \in_array($method, ["PUT", "MOVE", "COPY"])
+			&& \in_array($response->getStatusCode(), [Response::HTTP_CREATED, Response::HTTP_NO_CONTENT])
+			&& OcConfigHelper::getPostProcessingDelay() === 0
+		) {
+			if (\in_array($method, ["MOVE", "COPY"])) {
+				$url = $headers['Destination'];
+			}
+			WebDavHelper::waitForPostProcessingToFinish(
+				$url,
+				$user,
+				$password,
+				$headers,
+			);
+		}
 		return $response;
 	}
 
@@ -203,13 +255,6 @@ class HttpRequestHelper {
 		} else {
 			$debugResponses = false;
 		}
-		// use basic auth for 'public' user or no user
-		if ($user === 'public' || $user === null || $user === '') {
-			$bearerToken = null;
-		} else {
-			$useBearerToken = TokenHelper::useBearerToken();
-			$bearerToken = $useBearerToken ? TokenHelper::getTokens($user, $password, $url)['access_token'] : null;
-		}
 
 		$sendRetryLimit = self::numRetriesOnHttpTooEarly();
 		$sendCount = 0;
@@ -228,7 +273,6 @@ class HttpRequestHelper {
 				$stream,
 				$timeout,
 				$client,
-				$bearerToken,
 			);
 
 			if ($response->getStatusCode() >= 400
@@ -256,7 +300,8 @@ class HttpRequestHelper {
 				// we need to repeat the send request, because we got HTTP_TOO_EARLY or HTTP_CONFLICT
 				// wait 1 second before sending again, to give the server some time
 				// to finish whatever post-processing it might be doing.
-				self::debugResponse($response);
+				echo "[INFO] Received '" . $response->getStatusCode() .
+					"' status code, retrying request ($sendCount)...\n";
 				\sleep(1);
 			}
 		} while ($loopAgain);

tests/acceptance/TestHelpers/OcConfigHelper.php

@@ -30,6 +30,26 @@ use Psr\Http\Message\ResponseInterface;
  * A helper class for configuring OpenCloud server
  */
 class OcConfigHelper {
+	public static $postProcessingDelay = 0;
+
+	/**
+	 * @return int
+	 */
+	public static function getPostProcessingDelay(): int {
+		return self::$postProcessingDelay;
+	}
+
+	/**
+	 * @param string $postProcessingDelay
+	 *
+	 * @return void
+	 */
+	public static function setPostProcessingDelay(string $postProcessingDelay): void {
+		// extract number from string
+		$delay = (int) filter_var($postProcessingDelay, FILTER_SANITIZE_NUMBER_INT);
+		self::$postProcessingDelay = $delay;
+	}
+
 	/**
 	 * @param string $url
 	 * @param string $method

tests/acceptance/TestHelpers/TokenHelper.php

@@ -84,7 +84,9 @@ class TokenHelper {
 			$tokenData = [
 				'access_token' => $refreshedToken['access_token'],
 				'refresh_token' => $refreshedToken['refresh_token'],
-				'expires_at' => time() + 300 // 5 minutes
+				// set expiry to 240 (4 minutes) seconds to allow for some buffer
+				// token actually expires in 300 seconds (5 minutes)
+				'expires_at' => time() + 240
 			];
 			self::$tokenCache[$cacheKey] = $tokenData;
 			return $tokenData;
@@ -100,7 +102,9 @@ class TokenHelper {
 		$tokenData = [
 			'access_token' => $tokens['access_token'],
 			'refresh_token' => $tokens['refresh_token'],
-			'expires_at' => time() + 290 // set expiry to 290 seconds to allow for some buffer
+			// set expiry to 240 (4 minutes) seconds to allow for some buffer
+			// token actually expires in 300 seconds (5 minutes)
+			'expires_at' => time() + 240
 		];
 
 		// Save to cache

tests/acceptance/TestHelpers/WebDavHelper.php

@@ -923,4 +923,45 @@ class WebDavHelper {
 		$mtime = new DateTime($xmlPart[0]->__toString());
 		return $mtime->format('U');
 	}
+
+	/**
+	 * wait until the reqeust doesn't return 425 anymore
+	 *
+	 * @param string $url
+	 * @param ?string $user
+	 * @param ?string $password
+	 * @param ?array $headers
+	 *
+	 * @return void
+	 */
+	public static function waitForPostProcessingToFinish(
+		string $url,
+		?string $user = null,
+		?string $password = null,
+		?array $headers = [],
+	): void {
+		$retried = 0;
+		do {
+			$response = HttpRequestHelper::sendRequest(
+				$url,
+				'check-425-status',
+				'GET',
+				$user,
+				$password,
+				$headers,
+			);
+			$statusCode = $response->getStatusCode();
+			if ($statusCode !== 425) {
+				return;
+			}
+			$tryAgain = $retried < HttpRequestHelper::numRetriesOnHttpTooEarly();
+			if ($tryAgain) {
+				$retried += 1;
+				echo "[INFO] Waiting for post processing to finish, attempt ($retried)...\n";
+				// wait 1s and try again
+				\sleep(1);
+			}
+		} while ($tryAgain);
+		echo "[ERROR] 10 seconds timeout! Post processing did not finish in time.\n";
+	}
 }

tests/acceptance/bootstrap/FeatureContext.php

@@ -2026,8 +2026,12 @@ class FeatureContext extends BehatVariablesContext {
 		if ($response === null) {
 			$response = $this->getResponse();
 		}
+		$body = (string)$response->getBody();
+		if (!$body) {
+			return [];
+		}
 		return \json_decode(
-			(string)$response->getBody(),
+			$body,
 			true
 		);
 	}

tests/acceptance/bootstrap/OcConfigContext.php

@@ -68,6 +68,7 @@ class OcConfigContext implements Context {
 			$response->getStatusCode(),
 			"Failed to set async upload with delayed post processing"
 		);
+		OcConfigHelper::setPostProcessingDelay($delayTime);
 	}
 
 	/**
@@ -90,6 +91,9 @@ class OcConfigContext implements Context {
 			$response->getStatusCode(),
 			"Failed to set config $configVariable=$configValue"
 		);
+		if ($configVariable === "POSTPROCESSING_DELAY") {
+			OcConfigHelper::setPostProcessingDelay($configValue);
+		}
 	}
 
 	/**
@@ -184,6 +188,9 @@ class OcConfigContext implements Context {
 		$envs = [];
 		foreach ($table->getHash() as $row) {
 			$envs[$row['config']] = $row['value'];
+			if ($row['config'] === "POSTPROCESSING_DELAY") {
+				OcConfigHelper::setPostProcessingDelay($row['value']);
+			}
 		}
 
 		$response =  OcConfigHelper::reConfigureOc($envs);
@@ -200,6 +207,7 @@ class OcConfigContext implements Context {
 	 * @return void
 	 */
 	public function rollbackOc(): void {
+		OcConfigHelper::setPostProcessingDelay('0');
 		$response = OcConfigHelper::rollbackOc();
 		Assert::assertEquals(
 			200,

tests/acceptance/bootstrap/Provisioning.php

@@ -607,7 +607,7 @@ trait Provisioning {
 				Assert::assertEquals(
 					201,
 					$response->getStatusCode(),
-					__METHOD__ . " cannot create user '$userName' using Graph API.\nResponse:" .
+					__METHOD__ . " cannot create user '$userName'.\nResponse:" .
 					json_encode($this->getJsonDecodedResponse($response))
 				);
 
@@ -1083,7 +1083,7 @@ trait Provisioning {
 			Assert::assertEquals(
 				201,
 				$response->getStatusCode(),
-				__METHOD__ . " cannot create user '$user' using Graph API.\nResponse:" .
+				__METHOD__ . " cannot create user '$user'.\nResponse:" .
 				json_encode($this->getJsonDecodedResponse($response))
 			);
 			$userId = $this->getJsonDecodedResponse($response)['id'];

tests/acceptance/bootstrap/SpacesContext.php

@@ -750,6 +750,9 @@ class SpacesContext implements Context {
 		} else {
 			$rawBody =  $this->featureContext->getResponse()->getBody()->getContents();
 		}
+		if (!$rawBody) {
+			throw new Exception(__METHOD__ . " - Response body is empty");
+		}
 		$drives = json_decode($rawBody, true, 512, JSON_THROW_ON_ERROR);
 		if (isset($drives["value"])) {
 			$drives = $drives["value"];

tests/acceptance/bootstrap/WebDav.php

@@ -25,6 +25,7 @@ use GuzzleHttp\Exception\GuzzleException;
 use PHPUnit\Framework\Assert;
 use Psr\Http\Message\ResponseInterface;
 use GuzzleHttp\Stream\StreamInterface;
+use TestHelpers\OcConfigHelper;
 use TestHelpers\OcHelper;
 use TestHelpers\UploadHelper;
 use TestHelpers\WebDavHelper;
@@ -743,6 +744,7 @@ trait WebDav {
 
 	/**
 	 * @When the user waits for :time seconds for postprocessing to finish
+	 * @When the user waits for :time seconds
 	 *
 	 * @param int $time
 	 *
@@ -973,6 +975,61 @@ trait WebDav {
 		$this->checkDownloadedContentMatches($content, '', $response);
 	}
 
+	/**
+	 * check file content with retry
+	 *
+	 * @param string $user
+	 * @param string $fileName
+	 * @param string $content
+	 *
+	 * @return void
+	 * @throws Exception
+	 */
+	public function checkFileContentWithRetry(string $user, string $fileName, string $content): void {
+		$retried = 0;
+		do {
+			$tryAgain = false;
+			$response = $this->downloadFileAsUserUsingPassword($this->getActualUsername($user), $fileName);
+			$status = $response->getStatusCode();
+			$downloadedContent = $response->getBody()->getContents();
+			if ($status !== 200) {
+				$tryAgain = true;
+				$message = "Expected '200' but got '$status'";
+			} elseif ($downloadedContent !== $content) {
+				$tryAgain = true;
+				$message = "Expected content '$content' but got '$downloadedContent'";
+			}
+			$tryAgain = $tryAgain && $retried < HttpRequestHelper::numRetriesOnHttpTooEarly();
+			if ($tryAgain) {
+				$retried += 1;
+				echo "[INFO] File content mismatch. $message, checking content again ($retried)...\n";
+
+				// break the loop if status is 425 as the request will already be retried
+				if ($status === HttpRequestHelper::HTTP_TOO_EARLY) {
+					break;
+				}
+
+				// wait 1s and try again
+				\sleep(1);
+			}
+		} while ($tryAgain);
+		$this->theHTTPStatusCodeShouldBe(200, '', $response);
+		$this->checkDownloadedContentMatches($content, '', $response);
+	}
+
+	/**
+	 * @Then as :user the final content of file :fileName should be :content
+	 *
+	 * @param string $user
+	 * @param string $fileName
+	 * @param string $content
+	 *
+	 * @return void
+	 */
+	public function asUserFinalContentOfFileShouldBe(string $user, string $fileName, string $content): void {
+		$this->checkFileContentWithRetry($user, $fileName, $content);
+	}
+
 	/**
 	 * @Then /^the content of the following files for user "([^"]*)" should be "([^"]*)"$/
 	 *
@@ -2272,6 +2329,11 @@ trait WebDav {
 			"HTTP status code was not 201 or 204 while trying to upload file '$destination' for user '$user'",
 			$response
 		);
+
+		// check uploaded content only if post-processing delay is not configured
+		if (OcConfigHelper::getPostProcessingDelay() === 0) {
+			$this->checkFileContentWithRetry($user, $destination, $content);
+		}
 		return $response->getHeader('oc-fileid');
 	}
 

tests/acceptance/features/collaborativePosix/collaborativePosixFS.feature

@@ -29,7 +29,7 @@ Feature: create a resources using collaborative posixfs
   Scenario: create file
     When the administrator creates the file "test.txt" with content "content" for user "Alice" on the POSIX filesystem
     Then the command should be successful
-    And the content of file "/test.txt" for user "Alice" should be "content"
+    And as "Alice" the final content of file "test.txt" should be "content"
 
 
   Scenario: create large file
@@ -41,21 +41,22 @@ Feature: create a resources using collaborative posixfs
   Scenario: creates files sequentially in a folder
     When the administrator creates 50 files sequentially in the directory "firstFolder" for user "Alice" on the POSIX filesystem
     Then the command should be successful
-    And the content of file "/firstFolder/file_1.txt" for user "Alice" should be "file 1 content"
+    And as "Alice" the final content of file "/firstFolder/file_1.txt" should be "file 1 content"
-    And the content of file "/firstFolder/file_50.txt" for user "Alice" should be "file 50 content"
+    And as "Alice" the final content of file "/firstFolder/file_50.txt" should be "file 50 content"
 
 
   Scenario: creates files in parallel in a folder
     When the administrator creates 100 files in parallel in the directory "firstFolder" for user "Alice" on the POSIX filesystem
     Then the command should be successful
-    And the content of file "/firstFolder/parallel_1.txt" for user "Alice" should be "parallel file 1 content"
+    And as "Alice" the final content of file "/firstFolder/parallel_1.txt" should be "parallel file 1 content"
-    And the content of file "/firstFolder/parallel_100.txt" for user "Alice" should be "parallel file 100 content"
+    And as "Alice" the final content of file "/firstFolder/parallel_100.txt" should be "parallel file 100 content"
 
 
   Scenario: edit file
     Given user "Alice" has uploaded file with content "content" to "test.txt"
     When the administrator puts the content "new" into the file "test.txt" in the POSIX storage folder of user "Alice"
-    Then the content of file "/test.txt" for user "Alice" should be "contentnew"
+    Then the command should be successful
+    And as "Alice" the final content of file "test.txt" should be "contentnew"
 
 
   Scenario: read file content
@@ -68,14 +69,14 @@ Feature: create a resources using collaborative posixfs
     Given user "Alice" has uploaded file with content "content" to "test.txt"
     When the administrator copies the file "test.txt" to the folder "firstFolder" for user "Alice" on the POSIX filesystem
     Then the command should be successful
-    And the content of file "/firstFolder/test.txt" for user "Alice" should be "content"
+    And as "Alice" the final content of file "/firstFolder/test.txt" should be "content"
 
 
   Scenario: rename file
     Given user "Alice" has uploaded file with content "content" to "test.txt"
     When the administrator renames the file "test.txt" to "new-name.txt" for user "Alice" on the POSIX filesystem
     Then the command should be successful
-    And the content of file "/new-name.txt" for user "Alice" should be "content"
+    And as "Alice" the final content of file "/new-name.txt" should be "content"
 
 
   Scenario: check propfind after rename file
@@ -97,14 +98,14 @@ Feature: create a resources using collaborative posixfs
     Given the administrator has created the file "test.txt" with content "content" for user "Alice" on the POSIX filesystem
     When the administrator renames the file "test.txt" to "test.md" for user "Alice" on the POSIX filesystem
     Then the command should be successful
-    And the content of file "/test.md" for user "Alice" should be "content"
+    And as "Alice" the final content of file "/test.md" should be "content"
 
 
   Scenario: move file to folder
     Given user "Alice" has uploaded file with content "content" to "test.txt"
     When the administrator moves the file "test.txt" to the folder "firstFolder" for user "Alice" on the POSIX filesystem
     Then the command should be successful
-    And the content of file "/firstFolder/test.txt" for user "Alice" should be "content"
+    And as "Alice" the final content of file "/firstFolder/test.txt" should be "content"
     And as "Alice" file "/test.txt" should not exist
 
 
@@ -202,4 +203,4 @@ Feature: create a resources using collaborative posixfs
     And the administrator renames the file "test.txt" to "renamed.txt" for user "Alice" on the POSIX filesystem
     And the administrator checks the attribute "user.oc.name" of file "renamed.txt" for user "Alice" on the POSIX filesystem
     Then the command output should contain "renamed.txt"
-    And the content of file "/renamed.txt" for user "Alice" should be "content"
+    And as "Alice" the final content of file "/renamed.txt" should be "content"