[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>

Author: saw-jan

tests/acceptance/TestHelpers/HttpRequestHelper.php

@@ -33,6 +33,7 @@
 use Sabre\Xml\LibXMLException;
 use Sabre\Xml\Reader;
 use GuzzleHttp\Pool;
+use Symfony\Component\HttpFoundation\Response;
 
 /**
  * Helper for HTTP requests
@@ -74,7 +75,6 @@ public static function numRetriesOnHttpTooEarly(): int {
 	 *                     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 @@ public static function sendRequestOnce(
 		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 @@ public static function sendRequestOnce(
 		}
 
 		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 @@ public static function sendRequest(
 		} 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 @@ public static function sendRequest(
 				$stream,
 				$timeout,
 				$client,
-				$bearerToken,
 			);
 
 			if ($response->getStatusCode() >= 400
@@ -256,7 +300,8 @@ public static function sendRequest(
 				// 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 @@
  * 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 @@ public static function getTokens(string $username, string $password, string $url
 			$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 @@ public static function getTokens(string $username, string $password, string $url
 		$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 @@ public static function getMtimeOfResource(
 		$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 @@ public function getJsonDecodedResponse(?ResponseInterface $response = null): arr
 		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 @@ public function asyncUploadHasBeenEnabledWithDelayedPostProcessing(string $delay
 			$response->getStatusCode(),
 			"Failed to set async upload with delayed post processing"
 		);
+		OcConfigHelper::setPostProcessingDelay($delayTime);
 	}
 
 	/**
@@ -90,6 +91,9 @@ public function theConfigHasBeenSetTo(string $configVariable, string $configValu
 			$response->getStatusCode(),
 			"Failed to set config $configVariable=$configValue"
 		);
+		if ($configVariable === "POSTPROCESSING_DELAY") {
+			OcConfigHelper::setPostProcessingDelay($configValue);
+		}
 	}
 
 	/**
@@ -184,6 +188,9 @@ public function theConfigHasBeenSetToValue(TableNode $table): void {
 		$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 @@ public function theConfigHasBeenSetToValue(TableNode $table): void {
 	 * @return void
 	 */
 	public function rollbackOc(): void {
+		OcConfigHelper::setPostProcessingDelay('0');
 		$response = OcConfigHelper::rollbackOc();
 		Assert::assertEquals(
 			200,

tests/acceptance/bootstrap/Provisioning.php

@@ -607,7 +607,7 @@ public function usersHaveBeenCreated(
 				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 @@ public function userHasBeenCreated(
 			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 @@ public function rememberTheAvailableSpaces(?ResponseInterface $response = null):
 		} 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 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 @@ public function userDownloadsFileWithRangeUsingWebDavApi(string $user, string $f
 
 	/**
 	 * @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 @@ public function contentOfFileForUserShouldBe(string $fileName, string $user, str
 		$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 @@ public function userHasUploadedAFileWithContentTo(
 			"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');
 	}