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

[stable10] Refactor infoparser #34427

Merged
merged 4 commits into from Feb 11, 2019
Merged

[stable10] Refactor infoparser #34427

merged 4 commits into from Feb 11, 2019

Conversation

VicDeo
Copy link
Member

@VicDeo VicDeo commented Feb 8, 2019

Backport of #30457

Description

  • Throw exception in InfoParser instead of returning null on error
  • Log this exception in OC_App and rethrow it
  • Minor code cleanup
  • Some tests

Related Issue

#30190

Motivation and Context

Control everything ;)

How Has This Been Tested?

  1. By putting garbage inside appinfo and opening OC from web
  2. with php occ occ app:check-code for the app broken in the same manner

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@VicDeo VicDeo added this to the development milestone Feb 8, 2019
@VicDeo VicDeo self-assigned this Feb 8, 2019
lib/private/App/InfoParser.php Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Feb 8, 2019

Codecov Report

Merging #34427 into stable10 will increase coverage by 1.14%.
The diff coverage is 59.37%.

Impacted file tree graph

@@              Coverage Diff               @@
##             stable10   #34427      +/-   ##
==============================================
+ Coverage        63.5%   64.64%   +1.14%     
- Complexity      19140    19143       +3     
==============================================
  Files            1265     1202      -63     
  Lines           75296    68009    -7287     
  Branches         1291        0    -1291     
==============================================
- Hits            47813    43966    -3847     
+ Misses          27103    24043    -3060     
+ Partials          380        0     -380
Flag Coverage Δ Complexity Δ
#javascript ? ?
#phpunit 64.64% <59.37%> (+0.05%) 19143 <13> (+3) ⬆️
Impacted Files Coverage Δ Complexity Δ
core/register_command.php 0% <0%> (ø) 0 <0> (ø) ⬇️
lib/private/App/InfoParser.php 91.86% <100%> (+2.44%) 46 <11> (ø) ⬇️
lib/private/App/CodeChecker/InfoChecker.php 85% <100%> (+1.36%) 23 <1> (+1) ⬆️
lib/private/legacy/app.php 61.99% <25%> (-0.39%) 192 <0> (+1)
core/Command/App/CheckCode.php 37.07% <33.33%> (+37.07%) 21 <1> (ø) ⬇️
lib/private/App/AppManager.php 86.17% <33.33%> (-0.81%) 84 <0> (+1)
core/js/oc-dialogs.js
core/js/oc-backbone.js
settings/js/admin-apps.js
core/js/config.js
... and 61 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 da005c7...f76f9cf. Read the comment docs.

@VicDeo
Copy link
Member Author

VicDeo commented Feb 8, 2019

There were 2 failures:
1) Test\App\CodeChecker\InfoCheckerTest::testApps with data set #1 ('testapp-version', array())
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
+    0 => Array (...)
 )

/drone/src/tests/lib/App/CodeChecker/InfoCheckerTest.php:74
2) Test\App\CodeChecker\InfoCheckerTest::testApps with data set #4 ('testapp-version-missing', array())
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
+    0 => Array (...)
 )

ok, changes for InfoCheckerTest from #30845 were reverted which is wrong.

@owncloud owncloud deleted a comment from codecov bot Feb 11, 2019
@PVince81 PVince81 merged commit 6506e5f into stable10 Feb 11, 2019
@PVince81 PVince81 deleted the stable10-fix-30190 branch February 11, 2019 15:43
@PVince81 PVince81 modified the milestones: development, QA Apr 12, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Apr 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants