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

Move phpunit.xml to root folder of repo #273

Merged
merged 1 commit into from
Dec 5, 2018
Merged

Conversation

phil-davis
Copy link
Contributor

@phil-davis phil-davis commented Nov 22, 2018

Issue #272

  • Move phpunit.xml to root folder of repo
  • add PHP unit targets to Makefile
  • use those targets in drone
  • implement coding-standard checks

@codecov
Copy link

codecov bot commented Nov 23, 2018

Codecov Report

Merging #273 into master will decrease coverage by 1.36%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #273      +/-   ##
============================================
- Coverage      59.1%   57.74%   -1.37%     
+ Complexity      277      276       -1     
============================================
  Files            33       33              
  Lines          1274     1214      -60     
============================================
- Hits            753      701      -52     
+ Misses          521      513       -8
Impacted Files Coverage Δ Complexity Δ
templates/settings.php 0% <ø> (ø) 0 <0> (ø) ⬇️
lib/BackgroundScanner.php 51.11% <0%> (-5.46%) 19% <0%> (-1%)
lib/Status.php 72.72% <0%> (-2.28%) 26% <0%> (ø)
lib/AvirWrapper.php 86.27% <0%> (-1.23%) 24% <0%> (ø)
lib/Controller/SettingsController.php 88.46% <0%> (-0.83%) 6% <0%> (ø)
lib/Scanner/Local.php 92.3% <0%> (-0.8%) 5% <0%> (ø)
lib/Db/RuleMapper.php 94.95% <0%> (-0.73%) 8% <0%> (ø)
lib/AppConfig.php 81.66% <0%> (-0.6%) 21% <0%> (ø)
lib/Scanner/AbstractScanner.php 81.81% <0%> (-0.54%) 25% <0%> (ø)
appinfo/application.php 75.3% <0%> (-0.31%) 15% <0%> (ø)
... and 8 more

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 c166bac...449ff63. Read the comment docs.

@phil-davis
Copy link
Contributor Author

I made this a lot like the Makefile and CI of password_policy

Please review and see what else I should be adjusting.

Copy link
Member

@DeepDiver1975 DeepDiver1975 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really wonder why coverage is dropping .....

composer.json Outdated Show resolved Hide resolved
@phil-davis
Copy link
Contributor Author

@DeepDiver1975 @patrickjahns @PVince81 this is ready for review again.
I got rid of the local phpunit and it uses phpunit from core, in the same way that data_exporter is doing.

Copy link
Contributor

@patrickjahns patrickjahns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rules for phan/phpstan have already been added - though I don't see the composer files necessary for this

.gitignore Show resolved Hide resolved
@phil-davis
Copy link
Contributor Author

I pasted in the "standard" rules for "all" the make targets. Then they are there "ready to go".
@patrickjahns do you want me to remove them?

@phil-davis
Copy link
Contributor Author

I added coding standard matrix entry for PHP 5.6. This verifies that the code does not have any PHP7-specific stuff in it.

@phil-davis phil-davis force-pushed the move-phpunit-xml branch 2 times, most recently from 159a0e6 to 9ccebbd Compare November 27, 2018 16:49
@phil-davis
Copy link
Contributor Author

Rebased to trigger drone again.

@phil-davis
Copy link
Contributor Author

@patrickjahns @DeepDiver1975 drone is passing.
Ready for review again.

.drone.yml Outdated Show resolved Hide resolved
.drone.yml Outdated Show resolved Hide resolved
@patrickjahns
Copy link
Contributor

Thinking about https://github.com/owncloud/files_antivirus/pull/273/files/e6210e7ee74237f4e7b4ce77f2f49f2f513ae737#diff-4284501ea35da71f86e08fa8e56fdead
We probably require a rule that cleans up dependencies ( remove vendor / - vendor-bin//vendor - vendor-bin//composer.lock) folders to avoid platforming pinning when not necessary.

Opinions?

@phil-davis
Copy link
Contributor Author

We probably require a rule that cleans up dependencies ( remove vendor / - vendor-bin//vendor - vendor-bin//composer.lock) folders to avoid platforming pinning when not necessary.

Yes, that would be useful - specially when dropping back to PHP5.6 in a local dev environment to investigate weird stuff. I raised issue owncloud/QA#604 because this is a cross-app thing. We need to decide the clean* target names and what they do once, then implement in all relevant repos.

@phil-davis
Copy link
Contributor Author

@patrickjahns any more code changes needed for this PR?

.drone.yml Show resolved Hide resolved
@phil-davis
Copy link
Contributor Author

@patrickjahns after the little side discussion about PHP_VERSION=${PHP_VERSION} we are all sorted. Please review again and approve if happy.

@phil-davis
Copy link
Contributor Author

Today's "standard" CI-script changes have been made, like reference to tests/output/*.xml and content of .gitgnore. The previously long list of jumbled commits has been squashed. The scripts here should match the "new current style standard"

@patrickjahns @DeepDiver1975 ready for review again.

@phil-davis phil-davis force-pushed the move-phpunit-xml branch 2 times, most recently from 062210c to 2d13686 Compare December 4, 2018 03:40
@phil-davis
Copy link
Contributor Author

Rebased and resolved conflicts.
@patrickjahns @DeepDiver1975 please review. I like to be able to finish things.

@phil-davis
Copy link
Contributor Author

Rebased to get back in the drone queue - the previous drone CI had fails related to the corrupt core master QA tarball this morning.

@phil-davis phil-davis merged commit f92cd1a into master Dec 5, 2018
@delete-merged-branch delete-merged-branch bot deleted the move-phpunit-xml branch December 5, 2018 03:52
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.

4 participants