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

Declare testCase methods void to match modern phpunit declaration #34961

Closed
wants to merge 1 commit into from

Conversation

phil-davis
Copy link
Contributor

@phil-davis phil-davis commented Apr 4, 2019

Description

Add the void return type to test setUp() tearDown() etc methods of phpunit. This makes them compatible with PHPunit 8.0

Note: this will not backport to stable10 until PHP 5.6 and 7.0 support is dropped. Doing this in master also would mean that apps which inherit test classes from core would have to put the void return type in. And that would stop their unit tests from working against core stable10. It would be very annoying to have to make separate stable10 branches in lots of apps just for this! So merging this is "not a good thing" just yet.

Note: I was getting files_external_ftp unit tests to pass on Travis which has PHPunit 8.0, and used this just to see if the unit tests would pass - they do.

Motivation and Context

On Travis (for example) we started getting PHPunit 8.0 by default which puts return type declarations on some methods like function setUp(): void and so when we inherit from that, PHP complains if we do not also specify the return type void on the child class.

How Has This Been Tested?

CI and files_external_ftp unit tests with Travis

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

  • Backport (if applicable set "backport-request" label and remove when the backport was done)

@codecov
Copy link

codecov bot commented Apr 4, 2019

Codecov Report

Merging #34961 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #34961      +/-   ##
============================================
- Coverage     65.38%   65.38%   -0.01%     
  Complexity    18592    18592              
============================================
  Files          1213     1213              
  Lines         70408    70408              
  Branches       1295     1295              
============================================
- Hits          46036    46035       -1     
- Misses        23998    23999       +1     
  Partials        374      374
Flag Coverage Δ Complexity Δ
#javascript 52.85% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 66.83% <100%> (-0.01%) 18592 <8> (ø)
Impacted Files Coverage Δ Complexity Δ
apps/comments/tests/unit/Dav/CommentsNodeTest.php 96.41% <100%> (ø) 17 <1> (ø) ⬇️
...s/systemtags/tests/unit/activity/ExtensionTest.php 81.81% <100%> (ø) 3 <1> (ø) ⬇️
...pps/comments/tests/unit/Dav/CommentsPluginTest.php 100% <100%> (ø) 11 <1> (ø) ⬇️
...pps/comments/tests/unit/Dav/RootCollectionTest.php 98.38% <100%> (ø) 18 <1> (ø) ⬇️
apps/comments/tests/unit/ActivityListenerTest.php 100% <100%> (ø) 2 <1> (ø) ⬇️
...ps/systemtags/tests/unit/activity/ListenerTest.php 90.9% <100%> (ø) 5 <1> (ø) ⬇️
...mments/tests/unit/Dav/EntityTypeCollectionTest.php 100% <100%> (ø) 7 <1> (ø) ⬇️
...s/comments/tests/unit/Dav/EntityCollectionTest.php 100% <100%> (ø) 8 <1> (ø) ⬇️
apps/files_trashbin/lib/Expiration.php 96.55% <0%> (-1.73%) 29% <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 d8a0703...189b43d. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Apr 4, 2019

Codecov Report

Merging #34961 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #34961      +/-   ##
============================================
- Coverage     65.38%   65.38%   -0.01%     
  Complexity    18592    18592              
============================================
  Files          1213     1213              
  Lines         70408    70408              
  Branches       1295     1295              
============================================
- Hits          46036    46035       -1     
- Misses        23998    23999       +1     
  Partials        374      374
Flag Coverage Δ Complexity Δ
#javascript 52.85% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 66.83% <100%> (-0.01%) 18592 <8> (ø)
Impacted Files Coverage Δ Complexity Δ
apps/comments/tests/unit/Dav/CommentsNodeTest.php 96.41% <100%> (ø) 17 <1> (ø) ⬇️
...s/systemtags/tests/unit/activity/ExtensionTest.php 81.81% <100%> (ø) 3 <1> (ø) ⬇️
...pps/comments/tests/unit/Dav/CommentsPluginTest.php 100% <100%> (ø) 11 <1> (ø) ⬇️
...pps/comments/tests/unit/Dav/RootCollectionTest.php 98.38% <100%> (ø) 18 <1> (ø) ⬇️
apps/comments/tests/unit/ActivityListenerTest.php 100% <100%> (ø) 2 <1> (ø) ⬇️
...ps/systemtags/tests/unit/activity/ListenerTest.php 90.9% <100%> (ø) 5 <1> (ø) ⬇️
...mments/tests/unit/Dav/EntityTypeCollectionTest.php 100% <100%> (ø) 7 <1> (ø) ⬇️
...s/comments/tests/unit/Dav/EntityCollectionTest.php 100% <100%> (ø) 8 <1> (ø) ⬇️
apps/files_trashbin/lib/Expiration.php 96.55% <0%> (-1.73%) 29% <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 d8a0703...189b43d. Read the comment docs.

@codecov
Copy link

codecov bot commented Apr 4, 2019

Codecov Report

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

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #34961       +/-   ##
=============================================
- Coverage     67.18%   49.03%   -18.16%     
=============================================
  Files          1168      109     -1059     
  Lines         63575    10535    -53040     
  Branches          0     1289     +1289     
=============================================
- Hits          42716     5166    -37550     
+ Misses        20859     4991    -15868     
- Partials          0      378      +378
Flag Coverage Δ Complexity Δ
#javascript 53.7% <ø> (?) 0 <ø> (?)
#phpunit 38.59% <ø> (-28.6%) 0 <ø> (-18760)
Impacted Files Coverage Δ Complexity Δ
lib/private/Files/Storage/DAV.php 58.71% <0%> (-21.37%) 0% <0%> (ø)
apps/updatenotification/templates/admin.php
lib/private/Encryption/Keys/Storage.php
lib/private/App/CodeChecker/NodeVisitor.php
lib/private/RedisFactory.php
apps/dav/lib/Avatars/AvatarNode.php
...s/dav/appinfo/Migrations/Version20170202213905.php
apps/dav/lib/Upload/ChunkLocationProvider.php
apps/files/lib/AppInfo/Application.php
apps/systemtags/list.php
... and 1165 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 5ec7365...faf4c65. Read the comment docs.

@phil-davis phil-davis force-pushed the phpunit-declare-methods-void branch 3 times, most recently from 079ab77 to 7232f02 Compare May 27, 2019 13:00
@phil-davis phil-davis force-pushed the phpunit-declare-methods-void branch from 7232f02 to 230d59d Compare June 11, 2019 05:15
@phil-davis phil-davis force-pushed the phpunit-declare-methods-void branch 3 times, most recently from 8ae6dc8 to 7fa42d2 Compare June 25, 2019 05:12
@phil-davis phil-davis force-pushed the phpunit-declare-methods-void branch 7 times, most recently from 530d1db to 88283de Compare July 2, 2019 08:58
@phil-davis
Copy link
Contributor Author

Updated and rebased on 2019-07-02 and passing.
This is on-hold until we can drop PHP 7.0 in stable10 and then move up from PHPunit6 to PHPunit7.

@phil-davis phil-davis force-pushed the phpunit-declare-methods-void branch from 88283de to 1fa0ce8 Compare July 9, 2019 02:57
@phil-davis phil-davis force-pushed the phpunit-declare-methods-void branch 2 times, most recently from 6026ba0 to fdff142 Compare July 22, 2019 04:38
@phil-davis phil-davis force-pushed the phpunit-declare-methods-void branch from fdff142 to faf4c65 Compare July 30, 2019 04:47
@DeepDiver1975
Copy link
Member

As discussed in #35777 the master branch will from now on hold the ownCloud 10 codebase.

This PR targetted ownCloud 11 which is postponed to a far distant future.

Because of that I'm closing this PR and kindly ask you to re-submit this PR in a few days.

Thanks a lot for your patience

This was referenced Nov 30, 2019
@phil-davis phil-davis deleted the phpunit-declare-methods-void branch June 18, 2020 03:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants