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

Standardize CI and move unit tests to Unit folder #112

Merged
merged 3 commits into from
Dec 6, 2018
Merged

Conversation

phil-davis
Copy link
Contributor

@phil-davis phil-davis commented Dec 6, 2018

Issue #111

  • standardize Makefile and .drone.yml
  • move unit test code into tests/Unit folder (note that the folder has to have an uppercase U to keep the autoloader happy)
  • add phan and phpstan configs, and targets ot Makefile
  • fix the easy things reported by phan

@codecov
Copy link

codecov bot commented Dec 6, 2018

Codecov Report

Merging #112 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #112   +/-   ##
=========================================
  Coverage     99.23%   99.23%           
  Complexity       63       63           
=========================================
  Files             8        8           
  Lines           260      260           
=========================================
  Hits            258      258           
  Misses            2        2

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 28d448b...9e10d2f. Read the comment docs.

@codecov
Copy link

codecov bot commented Dec 6, 2018

Codecov Report

Merging #112 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #112   +/-   ##
=========================================
  Coverage     99.23%   99.23%           
  Complexity       63       63           
=========================================
  Files             8        8           
  Lines           260      260           
=========================================
  Hits            258      258           
  Misses            2        2
Impacted Files Coverage Δ Complexity Δ
lib/BackgroundJob.php 100% <100%> (ø) 7 <0> (ø) ⬇️
lib/Controller/PageController.php 100% <100%> (ø) 8 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 28d448b...b1eae3c. Read the comment docs.

@phil-davis
Copy link
Contributor Author

phil-davis commented Dec 6, 2018

phan reported:

make test-php-phan
php -d zend.enable_gc=0 vendor-bin/phan/vendor/bin/phan --config-file .phan/config.php --require-config-exists
[info] Disabling xdebug: Phan is around five times as slow when xdebug is enabled (xdebug only makes sense when debugging Phan itself)
[info] To run Phan with xdebug, set the environment variable PHAN_ALLOW_XDEBUG to 1.
[info] To disable this warning, set the environment variable PHAN_DISABLE_XDEBUG_WARN to 1.
[info] To include function signatures of xdebug, see .phan/internal_stubs/xdebug.phan_php
[debug] Checking PHAN_ALLOW_XDEBUG
[debug] The xdebug extension is loaded (2.6.1)
[debug] Process restarting (PHAN_ALLOW_XDEBUG=internal|2.6.1|1|*|*)
[debug] Running '/usr/bin/php7.1' '-n' '-c' '/tmp/76pW6Y' 'vendor-bin/phan/vendor/bin/phan' '--config-file' '.phan/config.php' '--require-config-exists'
lib/Activity/Extension.php:100 PhanTypeMismatchArgument Argument 1 (id) is string but \OCA\AnnouncementCenter\Manager::getAnnouncement() takes int defined at lib/Manager.php:110
lib/BackgroundJob.php:98 PhanTypeMismatchArgument Argument 2 (id) is int but \OCP\Notification\INotification::setObject() takes string defined at ../../lib/public/Notification/INotification.php:80
lib/Controller/PageController.php:156 PhanTypeMismatchArgument Argument 2 (id) is int but \OCP\Notification\INotification::setObject() takes string defined at ../../lib/public/Notification/INotification.php:80
lib/Notification/Notifier.php:77 PhanTypeMismatchArgument Argument 1 (id) is string but \OCA\AnnouncementCenter\Manager::getAnnouncement() takes int defined at lib/Manager.php:110
[debug] Restarted process exited 1
Makefile:133: recipe for target 'test-php-phan' failed
make: *** [test-php-phan] Error 1

BackgroundJob.php and PageController.php were easily fixed because we can reliably cast an int to a string in the reported calls to setObject()

For the other cases, a string (that hopefully always looks like an int) is being sent to a method that expects an int - those need to be looked into separately. See issue #113

@PVince81 PVince81 merged commit 0732c9c into master Dec 6, 2018
@delete-merged-branch delete-merged-branch bot deleted the standardize-CI branch December 6, 2018 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants