Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Standard infrastructure for CI and devs to make and test core and apps #582

Closed
phil-davis opened this issue Aug 15, 2018 · 7 comments
Closed
Assignees
Labels

Comments

@phil-davis
Copy link
Contributor

phil-davis commented Aug 15, 2018

Problem

At the moment:

  • the way to run different test types (code style, PHP/JS unit, API/webUI acceptance....) as a local dev and from automated CI has been done a bit differently from app to app and in core.
  • some things are run through the Makefile - make test-php-style etc. and other things need you to switch to some sub-directory and ./run.sh with a bunch of parameters and...

This makes it difficult:

  • as a developer, to keep in your head how to do these things, and even worse if the app you are working on does these things a bit different to core.
  • maintaining CI config files like .drone.yml - your head explodes after trying to apply the same sort of changes across multiple apps that have .drone.yml laid out differently.

Suggested standard way through the maze:

  • Makefile has knowledge of how to do all the build and test things.
  • developers run tests by typing make test-xxx
  • CI runs tests by doing make test-xxx commands from drone (or whatever CI system is in use next year...)
  • parameters to tests are given by putting env var/parameters on the make command, like the examples for unit tests https://doc.owncloud.org/server/10.0/developer_manual/core/unit-testing.html
  • Makefile is responsible for sorting out how to do it underneath
  • Makefile "gets dependencies sorted" in the sense that it does things like composer install for you as needed.
  • Makefile tells you if your system is missing some critical bit that would not be a dependency in composer or npm or... - e.g. if there is no php or no composer at all on your system. As a dev, you can install it one-time yourself.
  • a developer who is doing "routine" core or app development can run their tests just by knowing the make commands.
@phil-davis
Copy link
Contributor Author

@patrickjahns @individual-it @davitol @PVince81 @DeepDiver1975 and anyone else who cares to comment.

@patrickjahns
Copy link
Contributor

Things to add / keep in mind

CLI tools should be installed via composer-bin plugin in a separate namespace. That way we can ensure that they do not clog the dev dependencies of the application.

Example:
https://gist.github.com/patrickjahns/9d925cde469dda5c44fa8b9df5125e51

@patrickjahns
Copy link
Contributor

@phil-davis
Copy link
Contributor Author

phil-davis commented Dec 4, 2018

We are working through app Ci-scripts (Makefile composer.json .drone.yml vendor-bin stuff...) to do the following general things:

  • move any remaining Travis jobs to drone
  • implement make targets covering:
 Tests
--------------------------------------
test-php-unit:              Run php unit tests
test-php-unit-dbg:          Run php unit tests using phpdbg
test-php-style:             Run php-cs-fixer and check owncloud code-style
test-php-style-fix:         Run php-cs-fixer and fix code style issues
test-php-phan:              Run phan
test-php-phpstan:           Run phpstan
test-acceptance-api:        Run API acceptance tests
test-acceptance-cli:        Run CLI acceptance tests
test-acceptance-webui:      Run webUI acceptance tests
  • put the composer-bin-plugin into composer.json A minimal composer.json looks like:
{
  "name": "owncloud/my_app",
  "config" : {
    "platform": {
      "php": "5.6.37"
    }
  },
  "require": {
  },
  "require-dev": {
    "bamarni/composer-bin-plugin": "^1.2"
  },
  "extra": {
    "bamarni-bin": {
      "bin-links": false
    }
  }
}
  • add a vendor-bin folder
  • in vendor-bin/owncloud-codestyle/composer.json put:
{
  "require": {
    "owncloud/coding-standard": "^1.0"
  }
}
  • in vendor-bin/phan/composer.json put:
{
    "require": {
        "phan/phan": "^1.0.0"
    }
}
  • in vendor-bin/phpstan/composer.json put:
{
    "require": {
        "phpstan/phpstan": "^0.10.2"
    }
}
  • in .phan/config.php put a "standard" phan config (example is in testing app PR Implement phan stan testing#75 )
  • create .php_cs.dist with something like:
<?php

$config = new OC\CodingStandard\Config();

$config
	->setUsingCache(true)
	->getFinder()
	->exclude('l10n')
	->exclude('vendor')
	->exclude('vendor-bin')
	->notPath('/^c3.php/')
	->in(__DIR__);

return $config;
  • create phpstan.neon
parameters:
  bootstrap: %currentWorkingDirectory%/../../lib/base.php
  ignoreErrors:
  • adjust .gitignore to ignore stuff like:
.php_cs.cache

# Composer
vendor/
vendor-bin/**/vendor
vendor-bin/**/composer.lock

# Tests - auto-generated files
/tests/acceptance/output*
/tests/output

(among other stuff it already has)

  • add/modify .drone.yml to implement a "standard" sequence of pipeline steps and service sections for installing core, the app, fix permissions, run coding standard, phpunit, relevant acceptance test types (api, cli, webUI)
  • apply php-cs-fixer coding standard fixes
  • run owncloud-coding-standard in drone
  • remove any lint (from drone, Makefile``` ```composer ...) (php-cs-fixer will fail on lint/syntax errors)

Implementing phan and phpstan are for later. If an app already has them, great. If we are refactoring and these tools report just things that are obvious and easy to fix, then fix and implement the tool(s). Otherwise leave for a later PR.

@phil-davis
Copy link
Contributor Author

phil-davis commented Dec 4, 2018

To do for these apps:

Other apps that do not have Travis or drone or CI:
(listed here so we know they have been "observed" - giving them tests and CI would be a different job)

  • admin_audit
  • external
  • files_videoplayer
  • gallery (being superseded and current test framework is quite different)
  • theme-enterprise
  • user_external

@phil-davis
Copy link
Contributor Author

Closing this issue. All apps that can be converted have been done.
https://github.com/owncloud/sharepoint/issues/290 sharepoint is blocked. That issue can stay open, and be continued when a way forward is decided.

@PVince81
Copy link
Contributor

PVince81 commented Mar 7, 2019

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants