From 867da079a8adfd5a7f27b883cd9ce07ea15baaad Mon Sep 17 00:00:00 2001 From: Sawjan Gurung Date: Mon, 15 Dec 2025 14:45:12 +0545 Subject: [PATCH] [full-ci][tests-only] test: fix some test flakiness (#2003) * test: check content after upload Signed-off-by: Saw-jan test: check content with retry Signed-off-by: Saw-jan * test: check empty body before json decoding Signed-off-by: Saw-jan * test: wait post-processing for webdav requests if applicable Signed-off-by: Saw-jan * test: check token before doing request Signed-off-by: Saw-jan test: check body before json decoding Signed-off-by: Saw-jan test: add wait step Signed-off-by: Saw-jan --------- Signed-off-by: Saw-jan --- .../TestHelpers/HttpRequestHelper.php | 67 ++++++++++++++++--- .../acceptance/TestHelpers/OcConfigHelper.php | 20 ++++++ tests/acceptance/TestHelpers/TokenHelper.php | 8 ++- tests/acceptance/TestHelpers/WebDavHelper.php | 41 ++++++++++++ tests/acceptance/bootstrap/FeatureContext.php | 6 +- .../acceptance/bootstrap/OcConfigContext.php | 8 +++ tests/acceptance/bootstrap/Provisioning.php | 4 +- tests/acceptance/bootstrap/SpacesContext.php | 3 + tests/acceptance/bootstrap/WebDav.php | 62 +++++++++++++++++ .../collaborativePosixFS.feature | 23 ++++--- 10 files changed, 215 insertions(+), 27 deletions(-) diff --git a/tests/acceptance/TestHelpers/HttpRequestHelper.php b/tests/acceptance/TestHelpers/HttpRequestHelper.php index 51390a7140..5decf10599 100644 --- a/tests/acceptance/TestHelpers/HttpRequestHelper.php +++ b/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); diff --git a/tests/acceptance/TestHelpers/OcConfigHelper.php b/tests/acceptance/TestHelpers/OcConfigHelper.php index 188456a84f..8f06a7e004 100644 --- a/tests/acceptance/TestHelpers/OcConfigHelper.php +++ b/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 diff --git a/tests/acceptance/TestHelpers/TokenHelper.php b/tests/acceptance/TestHelpers/TokenHelper.php index 432dcad08a..7a57909a36 100644 --- a/tests/acceptance/TestHelpers/TokenHelper.php +++ b/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 diff --git a/tests/acceptance/TestHelpers/WebDavHelper.php b/tests/acceptance/TestHelpers/WebDavHelper.php index 01d36e4eef..14b1453342 100644 --- a/tests/acceptance/TestHelpers/WebDavHelper.php +++ b/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"; + } } diff --git a/tests/acceptance/bootstrap/FeatureContext.php b/tests/acceptance/bootstrap/FeatureContext.php index 7f0cfcebee..f44031eafd 100644 --- a/tests/acceptance/bootstrap/FeatureContext.php +++ b/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 ); } diff --git a/tests/acceptance/bootstrap/OcConfigContext.php b/tests/acceptance/bootstrap/OcConfigContext.php index 84d8cad5c3..e19aca8cf6 100644 --- a/tests/acceptance/bootstrap/OcConfigContext.php +++ b/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, diff --git a/tests/acceptance/bootstrap/Provisioning.php b/tests/acceptance/bootstrap/Provisioning.php index 6b48ac3fcd..9273c34f4f 100644 --- a/tests/acceptance/bootstrap/Provisioning.php +++ b/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']; diff --git a/tests/acceptance/bootstrap/SpacesContext.php b/tests/acceptance/bootstrap/SpacesContext.php index 2554583b74..2bd785707c 100644 --- a/tests/acceptance/bootstrap/SpacesContext.php +++ b/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"]; diff --git a/tests/acceptance/bootstrap/WebDav.php b/tests/acceptance/bootstrap/WebDav.php index e05aa733d3..620e6e36be 100644 --- a/tests/acceptance/bootstrap/WebDav.php +++ b/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'); } diff --git a/tests/acceptance/features/collaborativePosix/collaborativePosixFS.feature b/tests/acceptance/features/collaborativePosix/collaborativePosixFS.feature index 3d4cddd642..7e0088b4df 100644 --- a/tests/acceptance/features/collaborativePosix/collaborativePosixFS.feature +++ b/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 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_1.txt" should be "file 1 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 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_1.txt" should be "parallel file 1 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,28 +69,28 @@ 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: rename a created file 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 @@ -187,4 +188,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"