From 08e3d46adfcd561738d7a11a0fe44a053d249414 Mon Sep 17 00:00:00 2001
From: Phil Davis <phil@jankaritech.com>
Date: Tue, 9 May 2023 17:09:42 +0545
Subject: [PATCH] Cleanup code in TestHelpers

---
 tests/TestHelpers/Asserts/WebDav.php          | 116 +-----------------
 tests/TestHelpers/GraphHelper.php             |  11 +-
 tests/TestHelpers/HttpRequestHelper.php       |  20 +--
 tests/TestHelpers/OcisHelper.php              |  16 +--
 tests/TestHelpers/OcsApiHelper.php            |   4 +-
 tests/TestHelpers/SetupHelper.php             |  47 ++-----
 tests/TestHelpers/SharingHelper.php           |  13 +-
 tests/TestHelpers/TranslationHelper.php       |   2 +-
 tests/TestHelpers/UploadHelper.php            |  20 ++-
 tests/TestHelpers/UserHelper.php              |  26 ++--
 tests/TestHelpers/WebDavHelper.php            |  45 +++----
 .../bootstrap/PublicWebDavContext.php         |   4 +-
 .../acceptance/features/bootstrap/Sharing.php |   3 +-
 13 files changed, 101 insertions(+), 226 deletions(-)

diff --git a/tests/TestHelpers/Asserts/WebDav.php b/tests/TestHelpers/Asserts/WebDav.php
index 2ca6e14d8..8b72da2a2 100644
--- a/tests/TestHelpers/Asserts/WebDav.php
+++ b/tests/TestHelpers/Asserts/WebDav.php
@@ -21,15 +21,13 @@
  */
 namespace TestHelpers\Asserts;
 
-use Exception;
+use PHPUnit\Framework\Assert;
 use SimpleXMLElement;
-use TestHelpers\DownloadHelper;
-use TestHelpers\SetupHelper;
 
 /**
  * WebDAV related asserts
  */
-class WebDav extends \PHPUnit\Framework\Assert {
+class WebDav extends Assert {
 	/**
 	 *
 	 * @param string|null $element exception|message|reason
@@ -59,6 +57,8 @@ class WebDav extends \PHPUnit\Framework\Assert {
 			$result = $responseXml['value'][1]['value'];
 		} elseif ($element === "reason") {
 			$result = $responseXml['value'][3]['value'];
+		} else {
+			self::fail(__METHOD__ . " element must be one of exception, response or reason. But '$element' was passed in.");
 		}
 
 		self::assertEquals(
@@ -89,112 +89,4 @@ class WebDav extends \PHPUnit\Framework\Assert {
 			);
 		}
 	}
-
-	/**
-	 * Asserts that the content of a remote and a local file is the same
-	 * or is different
-	 *
-	 * @param string|null $baseUrl
-	 * @param string|null $username
-	 * @param string|null $password
-	 * @param string|null $remoteFile
-	 * @param string|null $localFile
-	 * @param string|null $xRequestId
-	 * @param bool $shouldBeSame (default true) if true then check that the file contents are the same
-	 *                           otherwise check that the file contents are different
-	 *
-	 * @return void
-	 */
-	public static function assertContentOfRemoteAndLocalFileIsSame(
-		?string $baseUrl,
-		?string $username,
-		?string $password,
-		?string $remoteFile,
-		?string $localFile,
-		?string $xRequestId = '',
-		?bool $shouldBeSame = true
-	):void {
-		$result = DownloadHelper::download(
-			$baseUrl,
-			$username,
-			$password,
-			$remoteFile,
-			$xRequestId
-		);
-
-		$localContent = \file_get_contents($localFile);
-		$downloadedContent = $result->getBody()->getContents();
-
-		if ($shouldBeSame) {
-			self::assertSame(
-				$localContent,
-				$downloadedContent
-			);
-		} else {
-			self::assertNotSame(
-				$localContent,
-				$downloadedContent
-			);
-		}
-	}
-
-	/**
-	 * Asserts that the content of a remote file (downloaded by DAV)
-	 * and a file in the skeleton folder of the system under test is the same
-	 * or is different
-	 *
-	 * @param string|null $baseUrl
-	 * @param string|null $username
-	 * @param string|null $password
-	 * @param string|null $adminUsername
-	 * @param string|null $adminPassword
-	 * @param string|null $remoteFile
-	 * @param string|null $fileInSkeletonFolder
-	 * @param string|null $xRequestId
-	 * @param bool $shouldBeSame (default true) if true then check that the file contents are the same
-	 *                           otherwise check that the file contents are different
-	 *
-	 * @return void
-	 * @throws Exception
-	 */
-	public static function assertContentOfDAVFileAndSkeletonFileOnSUT(
-		?string $baseUrl,
-		?string $username,
-		?string $password,
-		?string $adminUsername,
-		?string $adminPassword,
-		?string $remoteFile,
-		?string $fileInSkeletonFolder,
-		?string $xRequestId = '',
-		?bool $shouldBeSame = true
-	):void {
-		$result = DownloadHelper::download(
-			$baseUrl,
-			$username,
-			$password,
-			$remoteFile,
-			$xRequestId
-		);
-		$downloadedContent = $result->getBody()->getContents();
-
-		$localContent = SetupHelper::readSkeletonFile(
-			$fileInSkeletonFolder,
-			$xRequestId,
-			$baseUrl,
-			$adminUsername,
-			$adminPassword
-		);
-
-		if ($shouldBeSame) {
-			self::assertSame(
-				$localContent,
-				$downloadedContent
-			);
-		} else {
-			self::assertNotSame(
-				$localContent,
-				$downloadedContent
-			);
-		}
-	}
 }
diff --git a/tests/TestHelpers/GraphHelper.php b/tests/TestHelpers/GraphHelper.php
index 4c14b059c..b03445d9d 100644
--- a/tests/TestHelpers/GraphHelper.php
+++ b/tests/TestHelpers/GraphHelper.php
@@ -8,7 +8,6 @@
 
 namespace TestHelpers;
 
-use TestHelpers\HttpRequestHelper;
 use GuzzleHttp\Exception\GuzzleException;
 use Psr\Http\Message\RequestInterface;
 use Psr\Http\Message\ResponseInterface;
@@ -71,7 +70,7 @@ class GraphHelper {
 	 * @return string
 	 */
 	public static function separateAndGetValueForKey(string $keyName, array $actualDriveInformation): string {
-		// break the segment with @@@  to find the actual value from the actual drive information
+		// break the segment with @@@ to find the actual value from the actual drive information
 		$separatedKey = explode("@@@", $keyName);
 		// this stores the actual value of each key from drive information response used for assertion
 		$actualKeyValue = $actualDriveInformation;
@@ -107,7 +106,6 @@ class GraphHelper {
 	 * @param array|null $headers
 	 *
 	 * @return RequestInterface
-	 * @throws GuzzleException
 	 */
 	public static function createRequest(
 		string $baseUrl,
@@ -736,8 +734,10 @@ class GraphHelper {
 		$payload['onPremisesSamAccountName'] = $userName;
 		$payload['passwordProfile'] = ['password' => $password];
 		$payload['displayName'] = $displayName ?? $userName;
-		if (!empty($email)) {
-			$payload['mail'] = $email ?? $userName . '@example.com';
+		if (empty($email)) {
+			$payload['mail'] = $userName . '@example.com';
+		} else {
+			$payload['mail'] = $email;
 		}
 		$payload['accountEnabled'] = true;
 		return \json_encode($payload);
@@ -908,6 +908,7 @@ class GraphHelper {
 	 *
 	 * @return ResponseInterface
 	 *
+	 * @throws GuzzleException
 	 */
 	public static function getSingleSpace(
 		string $baseUrl,
diff --git a/tests/TestHelpers/HttpRequestHelper.php b/tests/TestHelpers/HttpRequestHelper.php
index 4911c510e..9d327e12c 100644
--- a/tests/TestHelpers/HttpRequestHelper.php
+++ b/tests/TestHelpers/HttpRequestHelper.php
@@ -42,10 +42,7 @@ use GuzzleHttp\Pool;
 class HttpRequestHelper {
 	public const HTTP_TOO_EARLY = 425;
 
-	/**
-	 * @var string
-	 */
-	private static $oCSelectorCookie = null;
+	private static ?string $oCSelectorCookie = null;
 
 	/**
 	 * @return string
@@ -65,7 +62,7 @@ class HttpRequestHelper {
 
 	/**
 	 * Some systems-under-test do async post-processing of operations like upload,
-	 * move etc. If a client does a request on the resource before the post-processing
+	 * move, etc. If a client does a request on the resource before the post-processing
 	 * is finished, then the server should return HTTP_TOO_EARLY "425". Clients are
 	 * expected to retry the request "some time later" (tm).
 	 *
@@ -124,9 +121,6 @@ class HttpRequestHelper {
 				$timeout
 			);
 		}
-		/**
-		 * @var RequestInterface $request
-		 */
 		$request = self::createRequest(
 			$url,
 			$xRequestId,
@@ -148,7 +142,7 @@ class HttpRequestHelper {
 		// The exceptions that might happen here include:
 		// ConnectException - in that case there is no response. Don't catch the exception.
 		// RequestException - if there is something in the response then pass it back.
-		//                    otherwise re-throw the exception.
+		//                    Otherwise, re-throw the exception.
 		// GuzzleException - something else unexpected happened. Don't catch the exception.
 		try {
 			$response = $client->send($request);
@@ -325,8 +319,7 @@ class HttpRequestHelper {
 		?array $requests,
 		?Client $client
 	):array {
-		$results = Pool::batch($client, $requests);
-		return $results;
+		return Pool::batch($client, $requests);
 	}
 
 	/**
@@ -364,8 +357,7 @@ class HttpRequestHelper {
 		$options['stream'] = $stream;
 		$options['verify'] = false;
 		$options['timeout'] = $timeout;
-		$client = new Client($options);
-		return $client;
+		return new Client($options);
 	}
 
 	/**
@@ -397,7 +389,7 @@ class HttpRequestHelper {
 			$headers['X-Request-ID'] = $xRequestId;
 		}
 		if (\is_array($body)) {
-			// when creating the client, it is possible to set 'form_params' and
+			// When creating the client, it is possible to set 'form_params' and
 			// the Client constructor sorts out doing this http_build_query stuff.
 			// But 'new Request' does not have the flexibility to do that.
 			// So we need to do it here.
diff --git a/tests/TestHelpers/OcisHelper.php b/tests/TestHelpers/OcisHelper.php
index 85be2d7ee..3232429b4 100644
--- a/tests/TestHelpers/OcisHelper.php
+++ b/tests/TestHelpers/OcisHelper.php
@@ -145,7 +145,7 @@ class OcisHelper {
 	 * @param string|null $destination
 	 *
 	 * @return void
-	 * @throws Exception
+	 * @throws Exception|GuzzleException
 	 */
 	public static function recurseUpload(
 		?string $baseUrl,
@@ -231,7 +231,7 @@ class OcisHelper {
 	 */
 	public static function getBaseDN():string {
 		$dn = \getenv("REVA_LDAP_BASE_DN");
-		return $dn ? $dn : "dc=owncloud,dc=com";
+		return $dn ?: "dc=owncloud,dc=com";
 	}
 
 	/**
@@ -239,7 +239,7 @@ class OcisHelper {
 	 */
 	public static function getGroupsOU():string {
 		$ou = \getenv("REVA_LDAP_GROUPS_OU");
-		return $ou ? $ou : "TestGroups";
+		return $ou ?: "TestGroups";
 	}
 
 	/**
@@ -247,7 +247,7 @@ class OcisHelper {
 	 */
 	public static function getUsersOU():string {
 		$ou = \getenv("REVA_LDAP_USERS_OU");
-		return $ou ? $ou : "TestUsers";
+		return $ou ?: "TestUsers";
 	}
 
 	/**
@@ -255,14 +255,14 @@ class OcisHelper {
 	 */
 	public static function getGroupSchema():string {
 		$schema = \getenv("REVA_LDAP_GROUP_SCHEMA");
-		return $schema ? $schema : "rfc2307";
+		return $schema ?: "rfc2307";
 	}
 	/**
 	 * @return string
 	 */
 	public static function getHostname():string {
 		$hostname = \getenv("REVA_LDAP_HOSTNAME");
-		return $hostname ? $hostname : "localhost";
+		return $hostname ?: "localhost";
 	}
 
 	/**
@@ -270,7 +270,7 @@ class OcisHelper {
 	 */
 	public static function getBindDN():string {
 		$dn = \getenv("REVA_LDAP_BIND_DN");
-		return $dn ? $dn : "cn=admin,dc=owncloud,dc=com";
+		return $dn ?: "cn=admin,dc=owncloud,dc=com";
 	}
 
 	/**
@@ -278,7 +278,7 @@ class OcisHelper {
 	 */
 	public static function getBindPassword():string {
 		$pw = \getenv("REVA_LDAP_BIND_PASSWORD");
-		return $pw ? $pw : "";
+		return $pw ?: "";
 	}
 
 	/**
diff --git a/tests/TestHelpers/OcsApiHelper.php b/tests/TestHelpers/OcsApiHelper.php
index 6e0623f42..27ba2d1d1 100644
--- a/tests/TestHelpers/OcsApiHelper.php
+++ b/tests/TestHelpers/OcsApiHelper.php
@@ -61,7 +61,7 @@ class OcsApiHelper {
 		if (\substr($fullUrl, -1) !== '/') {
 			$fullUrl .= '/';
 		}
-		$fullUrl .= "ocs/v{$ocsApiVersion}.php" . $path;
+		$fullUrl .= "ocs/v$ocsApiVersion.php" . $path;
 		$headers['OCS-APIREQUEST'] = true;
 		return HttpRequestHelper::sendRequest($fullUrl, $xRequestId, $method, $user, $password, $headers, $body);
 	}
@@ -91,7 +91,7 @@ class OcsApiHelper {
 		if (\substr($fullUrl, -1) !== '/') {
 			$fullUrl .= '/';
 		}
-		$fullUrl .= "ocs/v{$ocsApiVersion}.php" . $path;
+		$fullUrl .= "ocs/v$ocsApiVersion.php" . $path;
 		return HttpRequestHelper::createRequest(
 			$fullUrl,
 			$xRequestId,
diff --git a/tests/TestHelpers/SetupHelper.php b/tests/TestHelpers/SetupHelper.php
index 0a899ab8b..15c54866e 100644
--- a/tests/TestHelpers/SetupHelper.php
+++ b/tests/TestHelpers/SetupHelper.php
@@ -23,9 +23,7 @@ namespace TestHelpers;
 
 use Behat\Testwork\Hook\Scope\HookScope;
 use GuzzleHttp\Exception\GuzzleException;
-use GuzzleHttp\Exception\ServerException;
 use Exception;
-use Psr\Http\Message\ResponseInterface;
 use SimpleXMLElement;
 
 /**
@@ -35,22 +33,9 @@ use SimpleXMLElement;
  *
  */
 class SetupHelper extends \PHPUnit\Framework\Assert {
-	/**
-	 * @var string
-	 */
-	private static $ocPath = null;
-	/**
-	 * @var string
-	 */
-	private static $baseUrl = null;
-	/**
-	 * @var string
-	 */
-	private static $adminUsername = null;
-	/**
-	 * @var string
-	 */
-	private static $adminPassword = null;
+	private static ?string $baseUrl = null;
+	private static ?string $adminUsername = null;
+	private static ?string $adminPassword = null;
 
 	/**
 	 *
@@ -63,18 +48,6 @@ class SetupHelper extends \PHPUnit\Framework\Assert {
 			->getSettings() ['context'] ['parameters'];
 	}
 
-	/**
-	 * Fixup OC path so that it always starts with a "/" and does not end with
-	 * a "/".
-	 *
-	 * @param string|null $ocPath
-	 *
-	 * @return string
-	 */
-	private static function normaliseOcPath(?string $ocPath):string {
-		return '/' . \trim($ocPath, '/');
-	}
-
 	/**
 	 *
 	 * @param string|null $adminUsername
@@ -101,7 +74,6 @@ class SetupHelper extends \PHPUnit\Framework\Assert {
 		self::$adminUsername = $adminUsername;
 		self::$adminPassword = $adminPassword;
 		self::$baseUrl = \rtrim($baseUrl, '/');
-		self::$ocPath = self::normaliseOcPath($ocPath);
 	}
 
 	/**
@@ -113,6 +85,7 @@ class SetupHelper extends \PHPUnit\Framework\Assert {
 	 *
 	 * @return SimpleXMLElement
 	 * @throws GuzzleException
+	 * @throws Exception
 	 */
 	public static function getSysInfo(
 		?string $baseUrl,
@@ -315,6 +288,7 @@ class SetupHelper extends \PHPUnit\Framework\Assert {
 	 *
 	 * @return void
 	 * @throws GuzzleException
+	 * @throws Exception
 	 */
 	public static function createFileOnServer(
 		?string $filePathFromServerRoot,
@@ -421,13 +395,13 @@ class SetupHelper extends \PHPUnit\Framework\Assert {
 			$adminUsername,
 			$adminPassword,
 			'GET',
-			"/apps/testing/api/v1/file?file={$fileInCore}",
+			"/apps/testing/api/v1/file?file=$fileInCore",
 			$xRequestId
 		);
 		self::assertSame(
 			200,
 			$response->getStatusCode(),
-			"Failed to read the file {$fileInCore}"
+			"Failed to read the file $fileInCore"
 		);
 		$localContent = HttpRequestHelper::getResponseXml($response, __METHOD__);
 		$localContent = (string)$localContent->data->element->contentUrlEncoded;
@@ -470,17 +444,16 @@ class SetupHelper extends \PHPUnit\Framework\Assert {
 			$adminUsername,
 			$adminPassword,
 			'GET',
-			"/apps/testing/api/v1/file?file={$fileInSkeletonFolder}&absolute=true",
+			"/apps/testing/api/v1/file?file=$fileInSkeletonFolder&absolute=true",
 			$xRequestId
 		);
 		self::assertSame(
 			200,
 			$response->getStatusCode(),
-			"Failed to read the file {$fileInSkeletonFolder}"
+			"Failed to read the file $fileInSkeletonFolder"
 		);
 		$localContent = HttpRequestHelper::getResponseXml($response, __METHOD__);
 		$localContent = (string)$localContent->data->element->contentUrlEncoded;
-		$localContent = \urldecode($localContent);
-		return $localContent;
+		return \urldecode($localContent);
 	}
 }
diff --git a/tests/TestHelpers/SharingHelper.php b/tests/TestHelpers/SharingHelper.php
index 036772662..da3458a68 100644
--- a/tests/TestHelpers/SharingHelper.php
+++ b/tests/TestHelpers/SharingHelper.php
@@ -22,6 +22,7 @@
 namespace TestHelpers;
 
 use Exception;
+use GuzzleHttp\Exception\GuzzleException;
 use InvalidArgumentException;
 use Psr\Http\Message\ResponseInterface;
 use SimpleXMLElement;
@@ -52,7 +53,7 @@ class SharingHelper {
 		'accepted' => 0,
 		'pending' => 1,
 		'rejected' => 2,
-		'declined' => 2, // declined is a synonym for rejected
+		'declined' => 2, // declined is a synonym for rejected.
 	];
 
 	/**
@@ -88,7 +89,7 @@ class SharingHelper {
 	 * @param string $sharingApp
 	 *
 	 * @return ResponseInterface
-	 * @throws InvalidArgumentException
+	 * @throws InvalidArgumentException|GuzzleException
 	 */
 	public static function createShare(
 		string $baseUrl,
@@ -136,7 +137,7 @@ class SharingHelper {
 		if (\substr($fullUrl, -1) !== '/') {
 			$fullUrl .= '/';
 		}
-		$fullUrl .= "ocs/v{$ocsApiVersion}.php/apps/{$sharingApp}/api/v{$sharingApiVersion}/shares";
+		$fullUrl .= "ocs/v$ocsApiVersion.php/apps/$sharingApp/api/v$sharingApiVersion/shares";
 
 		$fd['path'] = $path;
 		$fd['shareType'] = self::getShareType($shareType);
@@ -145,7 +146,7 @@ class SharingHelper {
 			$fd['shareWith'] = $shareWith;
 		}
 		if ($publicUpload !== null) {
-			$fd['publicUpload'] = (bool) $publicUpload;
+			$fd['publicUpload'] = $publicUpload;
 		}
 		if ($sharePassword !== null) {
 			$fd['password'] = $sharePassword;
@@ -169,7 +170,7 @@ class SharingHelper {
 	}
 
 	/**
-	 * Calculates the permission sum (int) from given permissions.
+	 * Calculates the permission sum (int) from the given permissions.
 	 * Permissions can be passed in as int, string or array of int or string
 	 * 'read' => 1
 	 * 'update' => 2
@@ -197,7 +198,7 @@ class SharingHelper {
 			if (\array_key_exists($permission, self::PERMISSION_TYPES)) {
 				$permissionSum += self::PERMISSION_TYPES[$permission];
 			} elseif (\in_array($permission, self::PERMISSION_TYPES, true)) {
-				$permissionSum += (int) $permission;
+				$permissionSum += $permission;
 			} else {
 				throw new InvalidArgumentException(
 					"invalid permission type ($permission)"
diff --git a/tests/TestHelpers/TranslationHelper.php b/tests/TestHelpers/TranslationHelper.php
index bc262b473..388b4200a 100644
--- a/tests/TestHelpers/TranslationHelper.php
+++ b/tests/TestHelpers/TranslationHelper.php
@@ -25,7 +25,7 @@ namespace TestHelpers;
 /**
  * Class TranslationHelper
  *
- * Helper functions that are needed to run tests on different languages
+ * Helper functions that are needed to run tests in different languages
  *
  * @package TestHelpers
  */
diff --git a/tests/TestHelpers/UploadHelper.php b/tests/TestHelpers/UploadHelper.php
index f216db4b1..d5be3e19f 100644
--- a/tests/TestHelpers/UploadHelper.php
+++ b/tests/TestHelpers/UploadHelper.php
@@ -21,6 +21,8 @@
  */
 namespace TestHelpers;
 
+use GuzzleHttp\Exception\GuzzleException;
+use PHPUnit\Framework\Assert;
 use Psr\Http\Message\ResponseInterface;
 
 /**
@@ -29,7 +31,7 @@ use Psr\Http\Message\ResponseInterface;
  * @author Artur Neumann <artur@jankaritech.com>
  *
  */
-class UploadHelper extends \PHPUnit\Framework\Assert {
+class UploadHelper extends Assert {
 	/**
 	 *
 	 * @param string|null $baseUrl URL of owncloud
@@ -46,9 +48,10 @@ class UploadHelper extends \PHPUnit\Framework\Assert {
 	 * @param int|null $davPathVersionToUse (1|2)
 	 * @param int|null $chunkingVersion (1|2|null)
 	 *                                  if set to null chunking will not be used
-	 * @param int|null $noOfChunks how many chunks do we want to upload
+	 * @param int|null $noOfChunks how many chunks to upload
 	 *
 	 * @return ResponseInterface
+	 * @throws GuzzleException
 	 */
 	public static function upload(
 		?string $baseUrl,
@@ -79,10 +82,12 @@ class UploadHelper extends \PHPUnit\Framework\Assert {
 		} else {
 			//prepare chunking
 			$chunks = self::chunkFile($source, $noOfChunks);
-			$chunkingId = 'chunking-' . (string)\rand(1000, 9999);
+			$chunkingId = 'chunking-' . \rand(1000, 9999);
 			$v2ChunksDestination = '/uploads/' . $user . '/' . $chunkingId;
 		}
 
+		$result = null;
+
 		//prepare chunking version specific stuff
 		if ($chunkingVersion === 1) {
 			$headers['OC-Chunked'] = '1';
@@ -108,10 +113,11 @@ class UploadHelper extends \PHPUnit\Framework\Assert {
 		foreach ($chunks as $index => $chunk) {
 			if ($chunkingVersion === 1) {
 				$filename = $destination . "-" . $chunkingId . "-" .
-					\count($chunks) . '-' . ( string ) $index;
+					\count($chunks) . '-' . $index;
 				$davRequestType = "files";
-			} elseif ($chunkingVersion === 2) {
-				$filename = $v2ChunksDestination . '/' . (string)($index);
+			} else {
+				// do chunking version 2
+				$filename = $v2ChunksDestination . '/' . $index;
 				$davRequestType = "uploads";
 			}
 			$result = WebDavHelper::makeDavRequest(
@@ -152,6 +158,7 @@ class UploadHelper extends \PHPUnit\Framework\Assert {
 				return $result;
 			}
 		}
+		self::assertNotNull($result, __METHOD__ . " chunking version $chunkingVersion was requested but no upload was done.");
 		return $result;
 	}
 
@@ -169,6 +176,7 @@ class UploadHelper extends \PHPUnit\Framework\Assert {
 	 * @param string|null $exceptChunkingType empty string or "old" or "new"
 	 *
 	 * @return array of ResponseInterface
+	 * @throws GuzzleException
 	 */
 	public static function uploadWithAllMechanisms(
 		?string $baseUrl,
diff --git a/tests/TestHelpers/UserHelper.php b/tests/TestHelpers/UserHelper.php
index 8dda97fdc..33de75b37 100644
--- a/tests/TestHelpers/UserHelper.php
+++ b/tests/TestHelpers/UserHelper.php
@@ -23,6 +23,7 @@ namespace TestHelpers;
 
 use Exception;
 use GuzzleHttp\Exception\ClientException;
+use GuzzleHttp\Exception\GuzzleException;
 use Psr\Http\Message\ResponseInterface;
 
 /**
@@ -44,6 +45,7 @@ class UserHelper {
 	 * @param int|null $ocsApiVersion
 	 *
 	 * @return ResponseInterface
+	 * @throws GuzzleException
 	 */
 	public static function editUser(
 		?string $baseUrl,
@@ -99,15 +101,12 @@ class UserHelper {
 			$path = "/cloud/users/" . $data['user'];
 			$body = ["key" => $data['key'], 'value' => $data["value"]];
 			// Create the OCS API requests and push them to an array.
-			\array_push(
-				$requests,
-				OcsApiHelper::createOcsRequest(
-					$baseUrl,
-					'PUT',
-					$path,
-					$xRequestId,
-					$body
-				)
+			$requests[] = OcsApiHelper::createOcsRequest(
+				$baseUrl,
+				'PUT',
+				$path,
+				$xRequestId,
+				$body
 			);
 		}
 		// Send the array of requests at once in parallel.
@@ -135,6 +134,7 @@ class UserHelper {
 	 * @param int|null $ocsApiVersion
 	 *
 	 * @return ResponseInterface
+	 * @throws GuzzleException
 	 */
 	public static function getUser(
 		?string $baseUrl,
@@ -166,6 +166,7 @@ class UserHelper {
 	 * @param int|null $ocsApiVersion
 	 *
 	 * @return ResponseInterface
+	 * @throws GuzzleException
 	 */
 	public static function deleteUser(
 		?string $baseUrl,
@@ -196,6 +197,7 @@ class UserHelper {
 	 * @param string|null $xRequestId
 	 *
 	 * @return ResponseInterface
+	 * @throws GuzzleException
 	 */
 	public static function createGroup(
 		?string $baseUrl,
@@ -225,6 +227,7 @@ class UserHelper {
 	 * @param int|null $ocsApiVersion
 	 *
 	 * @return ResponseInterface
+	 * @throws GuzzleException
 	 */
 	public static function deleteGroup(
 		?string $baseUrl,
@@ -258,6 +261,7 @@ class UserHelper {
 	 * @param int|null $ocsApiVersion (1|2)
 	 *
 	 * @return ResponseInterface
+	 * @throws GuzzleException
 	 */
 	public static function addUserToGroup(
 		?string $baseUrl,
@@ -291,6 +295,7 @@ class UserHelper {
 	 * @param int|null $ocsApiVersion (1|2)
 	 *
 	 * @return ResponseInterface
+	 * @throws GuzzleException
 	 */
 	public static function removeUserFromGroup(
 		?string $baseUrl,
@@ -322,6 +327,7 @@ class UserHelper {
 	 * @param string|null $search
 	 *
 	 * @return ResponseInterface
+	 * @throws GuzzleException
 	 */
 	public static function getGroups(
 		?string $baseUrl,
@@ -349,7 +355,7 @@ class UserHelper {
 	 * @param string|null $search
 	 *
 	 * @return string[]
-	 * @throws Exception
+	 * @throws Exception|GuzzleException
 	 */
 	public static function getGroupsAsArray(
 		?string $baseUrl,
diff --git a/tests/TestHelpers/WebDavHelper.php b/tests/TestHelpers/WebDavHelper.php
index e8d0c9905..fdb249f74 100644
--- a/tests/TestHelpers/WebDavHelper.php
+++ b/tests/TestHelpers/WebDavHelper.php
@@ -29,7 +29,6 @@ use PHPUnit\Framework\Assert;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\StreamInterface;
 use DateTime;
-use TestHelpers\SpaceNotFoundException;
 
 /**
  * Helper to make WebDav Requests
@@ -41,12 +40,12 @@ class WebDavHelper {
 	public const DAV_VERSION_OLD = 1;
 	public const DAV_VERSION_NEW = 2;
 	public const DAV_VERSION_SPACES = 3;
-	public static $SPACE_ID_FROM_OCIS = '';
+	public static string $SPACE_ID_FROM_OCIS = '';
 
 	/**
-	 * @var array of users with their different spaces ids
+	 * @var array of users with their different space ids
 	 */
-	public static $spacesIdRef = [];
+	public static array $spacesIdRef = [];
 
 	/**
 	 * clear space id reference for user
@@ -75,7 +74,7 @@ class WebDavHelper {
 	 * @param int|null $davPathVersionToUse
 	 *
 	 * @return string
-	 * @throws Exception
+	 * @throws Exception|GuzzleException
 	 */
 	public static function getFileIdForPath(
 		?string $baseUrl,
@@ -174,7 +173,7 @@ class WebDavHelper {
 	 * @param string[] $properties
 	 *        string can contain namespace prefix,
 	 *        if no prefix is given 'd:' is used as prefix
-	 *        if associated array is used then the key will be used as namespace
+	 *        if an associative array is used, then the key will be used as namespace
 	 * @param string|null $xRequestId
 	 * @param string|null $folderDepth
 	 * @param string|null $type
@@ -182,6 +181,8 @@ class WebDavHelper {
 	 * @param string|null $doDavRequestAsUser
 	 *
 	 * @return ResponseInterface
+	 * @throws Exception
+	 * @throws GuzzleException
 	 */
 	public static function propfind(
 		?string $baseUrl,
@@ -237,6 +238,7 @@ class WebDavHelper {
 	 * @param string|null $type
 	 *
 	 * @return ResponseInterface
+	 * @throws GuzzleException
 	 */
 	public static function proppatch(
 		?string $baseUrl,
@@ -387,6 +389,7 @@ class WebDavHelper {
 	 * @param int|null $davPathVersionToUse
 	 *
 	 * @return ResponseInterface
+	 * @throws GuzzleException
 	 */
 	public static function listFolder(
 		?string $baseUrl,
@@ -544,7 +547,7 @@ class WebDavHelper {
 	}
 
 	/**
-	 * First checks if a user exist to return its space ID
+	 * First checks if a user exists to return its space ID
 	 * In case of any exception, it returns a fake space ID
 	 *
 	 * @param string $baseUrl
@@ -553,7 +556,7 @@ class WebDavHelper {
 	 * @param string $xRequestId
 	 *
 	 * @return string
-	 * @throws Exception
+	 * @throws Exception|GuzzleException
 	 */
 	public static function getPersonalSpaceIdForUserOrFakeIfNotFound(string $baseUrl, string $user, string $password, string $xRequestId):string {
 		try {
@@ -596,7 +599,7 @@ class WebDavHelper {
 	 * @param int|null $timeout
 	 * @param Client|null $client
 	 * @param array|null $urlParameter to concatenate with path
-	 * @param string|null $doDavRequestAsUser run the DAV as this user, if null its same as $user
+	 * @param string|null $doDavRequestAsUser run the DAV as this user, if null it is the same as $user
 	 *
 	 * @return ResponseInterface
 	 * @throws GuzzleException
@@ -766,7 +769,7 @@ class WebDavHelper {
 	}
 
 	/**
-	 * make sure there are no double slash in the URL
+	 * make sure there are no double-slashes in the URL
 	 *
 	 * @param string|null $url
 	 * @param bool|null $trailingSlash forces a trailing slash
@@ -779,16 +782,15 @@ class WebDavHelper {
 		} else {
 			$url = \rtrim($url, "/");
 		}
-		$url = \preg_replace("/([^:]\/)\/+/", '$1', $url);
-		return $url;
+		return \preg_replace("/([^:]\/)\/+/", '$1', $url);
 	}
 
 	/**
-	 * decides if the proposed dav version and chunking version are
+	 * Decides if the proposed dav version and chunking version are
 	 * a valid combination.
 	 * If no chunkingVersion is specified, then any dav version is valid.
 	 * If a chunkingVersion is specified, then it has to match the dav version.
-	 * Note: in future the dav and chunking versions might or might not
+	 * Note: in future, the dav and chunking versions might or might not
 	 * move together and/or be supported together. So a more complex
 	 * matrix could be needed here.
 	 *
@@ -821,9 +823,9 @@ class WebDavHelper {
 	 * @param int|null $davVersionToUse
 	 *
 	 * @return string
-	 * @throws Exception
+	 * @throws Exception|GuzzleException
 	 */
-	public static function getMtimeOfFileinPublicLinkShare(
+	public static function getMtimeOfFileInPublicLinkShare(
 		?string $baseUrl,
 		?string $fileName,
 		?string $token,
@@ -834,7 +836,7 @@ class WebDavHelper {
 			$baseUrl,
 			null,
 			null,
-			"/public-files/{$token}/{$fileName}",
+			"/public-files/$token/$fileName",
 			['d:getlastmodified'],
 			$xRequestId,
 			'1',
@@ -862,6 +864,7 @@ class WebDavHelper {
 	 *
 	 * @return string
 	 * @throws Exception
+	 * @throws GuzzleException
 	 */
 	public static function getMtimeOfResource(
 		?string $user,
@@ -886,13 +889,13 @@ class WebDavHelper {
 			$response,
 			__METHOD__
 		);
-		$xmlpart = $responseXmlObject->xpath("//d:getlastmodified");
+		$xmlPart = $responseXmlObject->xpath("//d:getlastmodified");
 		Assert::assertArrayHasKey(
 			0,
-			$xmlpart,
-			__METHOD__ . " XML part does not have key 0. Expected a value at index 0 of 'xmlPart' but, found: " . (string) json_encode($xmlpart)
+			$xmlPart,
+			__METHOD__ . " XML part does not have key 0. Expected a value at index 0 of 'xmlPart' but, found: " . json_encode($xmlPart)
 		);
-		$mtime = new DateTime($xmlpart[0]->__toString());
+		$mtime = new DateTime($xmlPart[0]->__toString());
 		return $mtime->format('U');
 	}
 }
diff --git a/tests/acceptance/features/bootstrap/PublicWebDavContext.php b/tests/acceptance/features/bootstrap/PublicWebDavContext.php
index 429d29969..1d0e96270 100644
--- a/tests/acceptance/features/bootstrap/PublicWebDavContext.php
+++ b/tests/acceptance/features/bootstrap/PublicWebDavContext.php
@@ -1428,7 +1428,7 @@ class PublicWebDavContext implements Context {
 		$mtime = \implode(" ", $mtime);
 		Assert::assertStringContainsString(
 			$mtime,
-			WebDavHelper::getMtimeOfFileinPublicLinkShare(
+			WebDavHelper::getMtimeOfFileInPublicLinkShare(
 				$baseUrl,
 				$fileName,
 				$token,
@@ -1454,7 +1454,7 @@ class PublicWebDavContext implements Context {
 		$baseUrl = $this->featureContext->getBaseUrl();
 		Assert::assertNotEquals(
 			$mtime,
-			WebDavHelper::getMtimeOfFileinPublicLinkShare(
+			WebDavHelper::getMtimeOfFileInPublicLinkShare(
 				$baseUrl,
 				$fileName,
 				$token,
diff --git a/tests/acceptance/features/bootstrap/Sharing.php b/tests/acceptance/features/bootstrap/Sharing.php
index 8116149a6..d4db525f7 100644
--- a/tests/acceptance/features/bootstrap/Sharing.php
+++ b/tests/acceptance/features/bootstrap/Sharing.php
@@ -1821,8 +1821,7 @@ trait Sharing {
 			true
 		);
 
-		Assert::assertEquals(
-			true,
+		Assert::assertTrue(
 			$this->isUserOrGroupInSharedData($group, "group", $permissions),
 			__METHOD__
 			. " Could not assert that user '$user' has shared '$filepath' with group '$group' with permissions '$permissions'"