Skip to content

Check for iterable responseXML->data in isUserOrGroupInSharedData#34707

Merged
phil-davis merged 1 commit into
masterfrom
check-for-iterable-responseXML-data
Mar 7, 2019
Merged

Check for iterable responseXML->data in isUserOrGroupInSharedData#34707
phil-davis merged 1 commit into
masterfrom
check-for-iterable-responseXML-data

Conversation

@phil-davis

@phil-davis phil-davis commented Mar 7, 2019

Copy link
Copy Markdown
Contributor

Description

In the acceptance test method isUserOrGroupInSharedData, if the responseXML data does not contain the expected array/iterable of sharing information, then the code can fall over with:

Warning: Invalid argument supplied for foreach() in /drone/src/tests/acceptance/features/bootstrap/Sharing.php line 834

Be nicer and emit an information message to say that the response data was not the expected type, and return false so that the caller can then "do its own thing" in response and report a more "normal" test fail message.

Motivation and Context

When doing a backport PR #34696 there was some problem with the backported code and the responses were not happening. It would be nice to have a clearer failure in the accceptance test output/

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 force-pushed the check-for-iterable-responseXML-data branch from 666cdcf to fbca9da Compare March 7, 2019 11:19
@codecov

codecov Bot commented Mar 7, 2019

Copy link
Copy Markdown

Codecov Report

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

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #34707       +/-   ##
=============================================
- Coverage     65.28%   48.39%   -16.89%     
=============================================
  Files          1210      109     -1101     
  Lines         69984    10493    -59491     
  Branches       1280     1280               
=============================================
- Hits          45688     5078    -40610     
+ Misses        23924     5043    -18881     
  Partials        372      372
Flag Coverage Δ Complexity Δ
#javascript 53.04% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 38.17% <ø> (-28.52%) 0 <ø> (-18481)
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 1093 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 bb21e34...fbca9da. Read the comment docs.

@codecov

codecov Bot commented Mar 7, 2019

Copy link
Copy Markdown

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #34707   +/-   ##
=========================================
  Coverage     65.28%   65.28%           
  Complexity    18481    18481           
=========================================
  Files          1210     1210           
  Lines         69984    69984           
  Branches       1280     1280           
=========================================
  Hits          45688    45688           
  Misses        23924    23924           
  Partials        372      372
Flag Coverage Δ Complexity Δ
#javascript 53.04% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 66.68% <ø> (ø) 18481 <ø> (ø) ⬇️

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 bb21e34...fbca9da. Read the comment docs.

@phil-davis

Copy link
Copy Markdown
Contributor Author

Backport stable10 #34709

@phil-davis

Copy link
Copy Markdown
Contributor Author

general drone problems at the moment. I will restart this when drone is sorted out.

@phil-davis phil-davis merged commit 9d0f23f into master Mar 7, 2019
@delete-merged-branch delete-merged-branch Bot deleted the check-for-iterable-responseXML-data branch March 7, 2019 14:41
@lock lock Bot locked as resolved and limited conversation to collaborators Mar 10, 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.

2 participants