Skip to content

Clone testing app into apps-external#35648

Merged
phil-davis merged 2 commits into
masterfrom
clone-testing-app-into-apps-external
Jun 26, 2019
Merged

Clone testing app into apps-external#35648
phil-davis merged 2 commits into
masterfrom
clone-testing-app-into-apps-external

Conversation

@phil-davis

@phil-davis phil-davis commented Jun 25, 2019

Copy link
Copy Markdown
Contributor

Description

Clone the testing app into the apps-external folder, which is where not-built-in apps should get installed anyway.

Adjust tests/lib/DB/MigrationsTest.php so that it does not use the testing app as an example app for its tests. Use another one of the built-in apps - I picked files_sharing.

Motivation and Context

The testing app was put into a separate repo a few months ago.
At the moment we are cloning it into the apps folder of core when running drone CI.
The PHP unit tests scan all code in the apps folder looking for PHP unit tests to run - see https://github.com/owncloud/core/blob/master/tests/apps.php loadDirectory()
That finds the apps/testing folder. So the unit tests in the testing app get run.

When doing core PRs that make non-compatible PHP unit test changes (e.g. bumping to a new major version of PHP unit, and implementing new features of PHP unit etc.), then the testing app unit tests sometimes fail, because they are from the separate testing app that has not had its unit test code updated yet.

How Has This Been Tested?

CI

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)

@phil-davis phil-davis self-assigned this Jun 25, 2019
@phil-davis
phil-davis force-pushed the clone-testing-app-into-apps-external branch from 5ae68db to d1e72c2 Compare June 25, 2019 06:50
@codecov

codecov Bot commented Jun 25, 2019

Copy link
Copy Markdown

Codecov Report

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

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #35648       +/-   ##
=============================================
- Coverage     65.68%    48.9%   -16.79%     
=============================================
  Files          1222      109     -1113     
  Lines         70935    10569    -60366     
  Branches       1289     1289               
=============================================
- Hits          46597     5169    -41428     
+ Misses        23960     5022    -18938     
  Partials        378      378
Flag Coverage Δ Complexity Δ
#javascript 53.7% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 38.29% <ø> (-28.78%) 0 <ø> (-18777)
Impacted Files Coverage Δ Complexity Δ
lib/private/Files/Storage/DAV.php 59.45% <0%> (-21.64%) 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 1104 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 0d356f5...d1e72c2. Read the comment docs.

@codecov

codecov Bot commented Jun 25, 2019

Copy link
Copy Markdown

Codecov Report

Merging #35648 into master will increase coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #35648      +/-   ##
============================================
+ Coverage     65.68%   65.73%   +0.04%     
  Complexity    18777    18777              
============================================
  Files          1222     1222              
  Lines         70935    70899      -36     
  Branches       1289     1289              
============================================
+ Hits          46596    46603       +7     
+ Misses        23961    23918      -43     
  Partials        378      378
Flag Coverage Δ Complexity Δ
#javascript 53.7% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 67.1% <ø> (+0.04%) 18777 <ø> (ø) ⬇️
Impacted Files Coverage Δ Complexity Δ
...aring/appinfo/Migrations/Version20170804201125.php 0% <0%> (ø) 2% <0%> (ø) ⬇️
lib/private/DB/MigrationService.php 90.44% <0%> (+5.14%) 53% <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 e22ef0f...0fff7bc. Read the comment docs.

@phil-davis
phil-davis force-pushed the clone-testing-app-into-apps-external branch from d1e72c2 to 82b69c4 Compare June 25, 2019 08:42
@phil-davis
phil-davis force-pushed the clone-testing-app-into-apps-external branch from 82b69c4 to 0fff7bc Compare June 26, 2019 10:30
@phil-davis
phil-davis marked this pull request as ready for review June 26, 2019 11:08

@individual-it individual-it left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

that should work fine, specially because now we have files-external created and configured automatically during installation

@phil-davis

Copy link
Copy Markdown
Contributor Author

Backport stable10 #35679

@phil-davis
phil-davis merged commit 9ad5e1e into master Jun 26, 2019
@delete-merged-branch
delete-merged-branch Bot deleted the clone-testing-app-into-apps-external branch June 26, 2019 13:04
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.

2 participants