From 6fd610f3ca959d95b07692fb8c7e2110b2910983 Mon Sep 17 00:00:00 2001 From: "sagargurung1001@gmail.com" Date: Tue, 6 Sep 2022 12:29:11 +0545 Subject: [PATCH 1/4] Exclude vendor-php dir for coding-standard-check --- .php-cs-fixer.dist.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php index 101395d99..e976ce47b 100644 --- a/.php-cs-fixer.dist.php +++ b/.php-cs-fixer.dist.php @@ -4,7 +4,8 @@ $dirToParse = 'tests/acceptance/'; $dirIterator = new DirectoryIterator(__DIR__ . '/' . $dirToParse); $excludeDirs = [ - 'node_modules' + 'node_modules', + 'vendor-php' ]; $finder = PhpCsFixer\Finder::create() From 1aabf51292a5c40ba17b6ddba83e50d9aa9e667a Mon Sep 17 00:00:00 2001 From: "sagargurung1001@gmail.com" Date: Tue, 6 Sep 2022 12:32:56 +0545 Subject: [PATCH 2/4] Fix all coding standard error --- Makefile | 2 +- .../features/bootstrap/GraphContext.php | 3 ++ .../features/bootstrap/SpacesContext.php | 42 +++++++++++++++---- .../features/bootstrap/SpacesTUSContext.php | 3 +- 4 files changed, 38 insertions(+), 12 deletions(-) diff --git a/Makefile b/Makefile index 49d9ac895..cf9b6526c 100644 --- a/Makefile +++ b/Makefile @@ -276,7 +276,7 @@ ci-format: $(BUILDIFIER) .PHONY: test-php-style test-php-style: vendor-bin/owncloud-codestyle/vendor vendor-bin/php_codesniffer/vendor $(PHP_CS_FIXER) fix -v --diff --allow-risky yes --dry-run - $(PHP_CODESNIFFER) --cache --runtime-set ignore_warnings_on_exit --standard=phpcs.xml tests/acceptance + $(PHP_CODESNIFFER) --cache --runtime-set ignore_warnings_on_exit --standard=phpcs.xml tests/acceptance tests/TestHelpers .PHONY: test-php-style-fix test-php-style-fix: vendor-bin/owncloud-codestyle/vendor diff --git a/tests/acceptance/features/bootstrap/GraphContext.php b/tests/acceptance/features/bootstrap/GraphContext.php index a2a3b5c33..caba60a1b 100644 --- a/tests/acceptance/features/bootstrap/GraphContext.php +++ b/tests/acceptance/features/bootstrap/GraphContext.php @@ -509,6 +509,7 @@ class GraphContext implements Context { * @param string $shouldOrNot (not|) * @param TableNode $table * + * @return void * @throws GuzzleException * @throws Exception */ @@ -539,10 +540,12 @@ class GraphContext implements Context { /** * @When /^the user "([^"]*)" changes its own password "([^"]*)" to "([^"]*)" using the Graph API$/ + * * @param string $user * @param string $currentPassword * @param string $newPassword * + * @return void * @throws GuzzleException * @throws Exception */ diff --git a/tests/acceptance/features/bootstrap/SpacesContext.php b/tests/acceptance/features/bootstrap/SpacesContext.php index f0840b64e..784269fc8 100644 --- a/tests/acceptance/features/bootstrap/SpacesContext.php +++ b/tests/acceptance/features/bootstrap/SpacesContext.php @@ -668,8 +668,8 @@ class SpacesContext implements Context { * * @param string $user * @param string $spaceName - * @return void * + * @return void * @throws GuzzleException */ public function theUserLooksUpTheSingleSpaceUsingTheGraphApiByUsingItsId(string $user, string $spaceName): void { @@ -847,8 +847,8 @@ class SpacesContext implements Context { * @param string $user * @param string $spaceName * @param string $foldersPath - * @return void * + * @return void * @throws GuzzleException */ public function theUserListsTheContentOfAPersonalSpaceRootUsingTheWebDAvApi( @@ -1006,6 +1006,7 @@ class SpacesContext implements Context { /** * @Then /^the json responded should contain a space "([^"]*)" (?:|(?:owned by|granted to) "([^"]*)" )(?:|(?:with description file|with space image) "([^"]*)" )with these key and value pairs:$/ + * * @param string $spaceName * @param string $userName * @param string $fileName @@ -1016,8 +1017,8 @@ class SpacesContext implements Context { */ public function jsonRespondedShouldContain( string $spaceName, - string $userName = '', - string $fileName = '', + string $userName, + string $fileName, TableNode $table ): void { $this->featureContext->verifyTableNodeColumns($table, ['key', 'value']); @@ -1090,8 +1091,8 @@ class SpacesContext implements Context { public function userHasSpaceWith( string $user, string $spaceName, - string $grantedUser = '', - string $fileName = '', + string $grantedUser, + string $fileName, TableNode $table ): void { $this->theUserListsAllHisAvailableSpacesUsingTheGraphApi($user); @@ -1292,6 +1293,7 @@ class SpacesContext implements Context { * and returns found search results if found else returns false * * @param string|null $entryNameToSearch + * @param string $folderPath * * @return array * string if $entryNameToSearch is given and is found @@ -1545,6 +1547,7 @@ class SpacesContext implements Context { * @param string $user * @param string $spaceName * @param string $newDescription + * * @return void * @throws GuzzleException * @throws JsonException @@ -2496,7 +2499,7 @@ class SpacesContext implements Context { /** * User get all objects in the trash of project space * - * method "getTrashbinContentFromResponseXml" borrowed from core repository + * Method "getTrashbinContentFromResponseXml" borrowed from core repository * and return array like: * [1] => Array * ( @@ -2619,6 +2622,7 @@ class SpacesContext implements Context { * @param string $width * @param string $height * + * @return void * @throws GuzzleException */ public function downloadPreview( @@ -2659,6 +2663,7 @@ class SpacesContext implements Context { * @param string $fileName * @param string $spaceName * + * @return void * @throws GuzzleException */ public function downloadFile( @@ -2689,6 +2694,7 @@ class SpacesContext implements Context { * @param string $path * @param string $spaceName * + * @return void * @throws GuzzleException */ public function userRequestsTheChecksumViaPropfindInSpace( @@ -2726,6 +2732,7 @@ class SpacesContext implements Context { * @param string $path * @param string $spaceName * + * @return void * @throws GuzzleException */ public function userUploadsFileWithChecksumWithContentInSpace( @@ -2758,6 +2765,7 @@ class SpacesContext implements Context { * @param string $index * @param string $spaceName * + * @return void * @throws GuzzleException */ public function downloadVersionOfTheFile( @@ -2903,6 +2911,9 @@ class SpacesContext implements Context { /** * @Then /^these etags should have changed$/ * + * @param TableNode $table + * + * @return void * @throws GuzzleException */ public function theseEtagsShouldHaveChanged(TableNode $table): void { @@ -2930,6 +2941,7 @@ class SpacesContext implements Context { * * @param TableNode $table * + * @return void * @throws GuzzleException */ public function theseEtagsShouldNotHaveChanged(TableNode $table): void { @@ -2955,6 +2967,11 @@ class SpacesContext implements Context { /** * @Given /^user "([^"]*)" has stored etag of element "([^"]*)" inside space "([^"]*)"$/ * + * @param string $user + * @param string $path + * @param string $space + * + * @return void * @throws GuzzleException | Exception */ public function userHasStoredEtagOfElementFromSpace(string $user, string $path, string $space):void { @@ -2972,9 +2989,15 @@ class SpacesContext implements Context { /** * @Given /^user "([^"]*)" has stored etag of element "([^"]*)" on path "([^"]*)" inside space "([^"]*)"$/ * + * @param $user + * @param $path + * @param $storePath + * @param $space + * + * @return void * @throws Exception | GuzzleException */ - public function userHasStoredEtagOfElementOnPathFromSpace($user, $path, $storePath, $space) { + public function userHasStoredEtagOfElementOnPathFromSpace($user, $path, $storePath, $space):void { $user = $this->featureContext->getActualUsername($user); $this->storeEtagOfElementInSpaceForUser( $user, @@ -2995,9 +3018,10 @@ class SpacesContext implements Context { * @param TableNode $properties * @param string $spaceName * + * @return void * @throws Exception | GuzzleException */ - public function userHasLockedResourceOfSpace(string $user, string $resource, TableNode $properties, string $spaceName) { + public function userHasLockedResourceOfSpace(string $user, string $resource, TableNode $properties, string $spaceName): void { $this->setSpaceIDByName($user, $spaceName); $this->webDavLockingContext->lockFileUsingWebDavAPI($user, $resource, $properties); } diff --git a/tests/acceptance/features/bootstrap/SpacesTUSContext.php b/tests/acceptance/features/bootstrap/SpacesTUSContext.php index 45bbc065e..700776977 100644 --- a/tests/acceptance/features/bootstrap/SpacesTUSContext.php +++ b/tests/acceptance/features/bootstrap/SpacesTUSContext.php @@ -85,12 +85,11 @@ class SpacesTUSContext implements Context { * * @param string $user * @param string $source - * @param string $spaceName * @param string $destination + * @param string $spaceName * @param array|null $uploadMetadata * * @return void - * * @throws Exception * @throws GuzzleException */ From dbf695667699d06eb7933de161c6d27e50ccdd0a Mon Sep 17 00:00:00 2001 From: "sagargurung1001@gmail.com" Date: Tue, 6 Sep 2022 15:37:04 +0545 Subject: [PATCH 3/4] Added coding-standard-php pipeline in drone --- .drone.star | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++++ Makefile | 6 ++++ 2 files changed, 107 insertions(+) diff --git a/.drone.star b/.drone.star index 2ad14a48f..702450686 100644 --- a/.drone.star +++ b/.drone.star @@ -148,6 +148,10 @@ config = { "architectures": ["arm", "arm64", "amd64"], }, "litmus": True, + "codestyle": True, + "branches": [ + "master", + ], } # volume for steps to cache Go dependencies between steps of a pipeline @@ -226,6 +230,7 @@ def main(ctx): cancelPreviousBuilds() + \ buildWebCache(ctx) + \ [buildOcisBinaryForTesting(ctx)] + \ + codestyle(ctx) + \ cacheCoreReposForTesting(ctx) + \ testOcisModules(ctx) + \ testPipelines(ctx) @@ -560,6 +565,102 @@ def uploadScanResults(ctx): }, } +def vendorbinCodestyle(phpVersion): + return [{ + "name": "vendorbin-codestyle", + "image": OC_CI_PHP % phpVersion, + "environment": { + "COMPOSER_HOME": "%s/.cache/composer" % dirs["base"], + }, + "commands": [ + "make vendor-bin-codestyle", + ], + }] + +def vendorbinCodesniffer(phpVersion): + return [{ + "name": "vendorbin-codesniffer", + "image": OC_CI_PHP % phpVersion, + "environment": { + "COMPOSER_HOME": "%s/.cache/composer" % dirs["base"], + }, + "commands": [ + "make vendor-bin-codesniffer", + ], + }] + +def codestyle(ctx): + pipelines = [] + + if "codestyle" not in config: + return pipelines + + default = { + "phpVersions": [DEFAULT_PHP_VERSION], + } + + if "defaults" in config: + if "codestyle" in config["defaults"]: + for item in config["defaults"]["codestyle"]: + default[item] = config["defaults"]["codestyle"][item] + + codestyleConfig = config["codestyle"] + + if type(codestyleConfig) == "bool": + if codestyleConfig: + # the config has 'codestyle' true, so specify an empty dict that will get the defaults + codestyleConfig = {} + else: + return pipelines + + if len(codestyleConfig) == 0: + # 'codestyle' is an empty dict, so specify a single section that will get the defaults + codestyleConfig = {"doDefault": {}} + + for category, matrix in codestyleConfig.items(): + params = {} + for item in default: + params[item] = matrix[item] if item in matrix else default[item] + + for phpVersion in params["phpVersions"]: + name = "coding-standard-php%s" % phpVersion + + result = { + "kind": "pipeline", + "type": "docker", + "name": name, + "workspace": { + "base": "/drone", + "path": "src", + }, + "steps": skipIfUnchanged(ctx, "acceptance-tests") + + vendorbinCodestyle(phpVersion) + + vendorbinCodesniffer(phpVersion) + + [ + { + "name": "php-style", + "image": OC_CI_PHP % phpVersion, + "commands": [ + "make test-php-style", + ], + }, + ], + "depends_on": [], + "trigger": { + "ref": [ + "refs/pull/**", + "refs/tags/**", + ], + }, + } + + for branch in config["branches"]: + result["trigger"]["ref"].append("refs/heads/%s" % branch) + + pipelines.append(result) + + return pipelines + def localApiTests(ctx, storage, suite, accounts_hash_difficulty = 4): early_fail = config["localApiTests"]["earlyFail"] if "earlyFail" in config["localApiTests"] else False diff --git a/Makefile b/Makefile index cf9b6526c..d4e43e255 100644 --- a/Makefile +++ b/Makefile @@ -283,6 +283,12 @@ test-php-style-fix: vendor-bin/owncloud-codestyle/vendor $(PHP_CS_FIXER) fix -v --diff --allow-risky yes $(PHP_CODEBEAUTIFIER) --cache --runtime-set ignore_warnings_on_exit --standard=phpcs.xml tests/acceptance +.PHONY: vendor-bin-codestyle +vendor-bin-codestyle: vendor-bin/owncloud-codestyle/vendor + +.PHONY: vendor-bin-codesniffer +vendor-bin-codesniffer: vendor-bin/php_codesniffer/vendor + vendor-bin/owncloud-codestyle/vendor: vendor/bamarni/composer-bin-plugin vendor-bin/owncloud-codestyle/composer.lock composer bin owncloud-codestyle install --no-progress From 61b9b05e40b408d1fd30a894b3513504ad3d10e7 Mon Sep 17 00:00:00 2001 From: "sagargurung1001@gmail.com" Date: Tue, 6 Sep 2022 15:58:42 +0545 Subject: [PATCH 4/4] Check coding standard with force error --- .drone.star | 29 +++++++------------ .../features/bootstrap/SpacesContext.php | 26 ++++++++--------- 2 files changed, 24 insertions(+), 31 deletions(-) diff --git a/.drone.star b/.drone.star index 702450686..04b0fefcd 100644 --- a/.drone.star +++ b/.drone.star @@ -149,9 +149,6 @@ config = { }, "litmus": True, "codestyle": True, - "branches": [ - "master", - ], } # volume for steps to cache Go dependencies between steps of a pipeline @@ -228,9 +225,9 @@ def main(ctx): test_pipelines = \ cancelPreviousBuilds() + \ + codestyle(ctx) + \ buildWebCache(ctx) + \ [buildOcisBinaryForTesting(ctx)] + \ - codestyle(ctx) + \ cacheCoreReposForTesting(ctx) + \ testOcisModules(ctx) + \ testPipelines(ctx) @@ -593,7 +590,7 @@ def codestyle(ctx): pipelines = [] if "codestyle" not in config: - return pipelines + return [] default = { "phpVersions": [DEFAULT_PHP_VERSION], @@ -633,7 +630,7 @@ def codestyle(ctx): "base": "/drone", "path": "src", }, - "steps": skipIfUnchanged(ctx, "acceptance-tests") + + "steps": skipIfUnchanged(ctx, "lint") + vendorbinCodestyle(phpVersion) + vendorbinCodesniffer(phpVersion) + [ @@ -648,15 +645,13 @@ def codestyle(ctx): "depends_on": [], "trigger": { "ref": [ + "refs/heads/master", "refs/pull/**", "refs/tags/**", ], }, } - for branch in config["branches"]: - result["trigger"]["ref"].append("refs/heads/%s" % branch) - pipelines.append(result) return pipelines @@ -2109,17 +2104,15 @@ def skipIfUnchanged(ctx, type): ] skip = [] - if type == "acceptance-tests": + if type == "acceptance-tests" or type == "e2e-tests" or type == "lint": skip = base + unit - if type == "unit-tests": + elif type == "unit-tests": skip = base + acceptance - if type == "build-binary" or type == "build-docker" or type == "litmus": + elif type == "build-binary" or type == "build-docker" or type == "litmus": skip = base + unit + acceptance - if type == "cache": + elif type == "cache": skip = base - if type == "e2e-tests": - skip = base + unit - if len(skip) == 0: + else: return [] return [{ @@ -3010,7 +3003,7 @@ def restoreWebE2EYarnCache(): "mc cp -r -a s3/$CACHE_BUCKET/ocis/web-test-runner/$WEB_COMMITID/e2e.tar.gz %s" % dirs["zip"], ], }, { - # we need to install again becase the node_modules are not cached + # we need to install again because the node_modules are not cached "name": "unzip-and-install-yarn-e2e", "image": OC_CI_NODEJS % DEFAULT_NODEJS_VERSION, "commands": [ @@ -3032,7 +3025,7 @@ def restoreWebAcceptanceYarnCache(): "mc cp -r -a s3/$CACHE_BUCKET/ocis/web-test-runner/$WEB_COMMITID/acceptance.tar.gz %s" % dirs["zip"], ], }, { - # we need to install again becase the node_modules are not cached + # we need to install again because the node_modules are not cached "name": "unzip-and-install-yarn-acceptance", "image": OC_CI_NODEJS % DEFAULT_NODEJS_VERSION, "commands": [ diff --git a/tests/acceptance/features/bootstrap/SpacesContext.php b/tests/acceptance/features/bootstrap/SpacesContext.php index 784269fc8..6b8004a9f 100644 --- a/tests/acceptance/features/bootstrap/SpacesContext.php +++ b/tests/acceptance/features/bootstrap/SpacesContext.php @@ -1008,8 +1008,8 @@ class SpacesContext implements Context { * @Then /^the json responded should contain a space "([^"]*)" (?:|(?:owned by|granted to) "([^"]*)" )(?:|(?:with description file|with space image) "([^"]*)" )with these key and value pairs:$/ * * @param string $spaceName - * @param string $userName - * @param string $fileName + * @param string|null $userName + * @param string|null $fileName * @param TableNode $table * * @return void @@ -1017,8 +1017,8 @@ class SpacesContext implements Context { */ public function jsonRespondedShouldContain( string $spaceName, - string $userName, - string $fileName, + ?string $userName = null, + ?string $fileName = null, TableNode $table ): void { $this->featureContext->verifyTableNodeColumns($table, ['key', 'value']); @@ -1081,8 +1081,8 @@ class SpacesContext implements Context { * * @param string $user * @param string $spaceName - * @param string $grantedUser - * @param string $fileName + * @param string|null $grantedUser + * @param string|null $fileName * @param TableNode $table * * @return void @@ -1091,8 +1091,8 @@ class SpacesContext implements Context { public function userHasSpaceWith( string $user, string $spaceName, - string $grantedUser, - string $fileName, + ?string $grantedUser = null, + ?string $fileName = null, TableNode $table ): void { $this->theUserListsAllHisAvailableSpacesUsingTheGraphApi($user); @@ -2989,15 +2989,15 @@ class SpacesContext implements Context { /** * @Given /^user "([^"]*)" has stored etag of element "([^"]*)" on path "([^"]*)" inside space "([^"]*)"$/ * - * @param $user - * @param $path - * @param $storePath - * @param $space + * @param string $user + * @param string $path + * @param string $storePath + * @param string $space * * @return void * @throws Exception | GuzzleException */ - public function userHasStoredEtagOfElementOnPathFromSpace($user, $path, $storePath, $space):void { + public function userHasStoredEtagOfElementOnPathFromSpace(string $user, string $path, string $storePath, string $space):void { $user = $this->featureContext->getActualUsername($user); $this->storeEtagOfElementInSpaceForUser( $user,