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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add cli tests for config file values #35269

Merged
merged 1 commit into from May 23, 2019
Merged

Add cli tests for config file values #35269

merged 1 commit into from May 23, 2019

Conversation

paurakhsharma
Copy link
Member

Description

Add cli tests for checking config.php default values.

Related Issue

#31347

Motivation and Context

How Has This Been Tested?

馃

Screenshots (if appropriate):

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)
  • Technical debt
  • Tests

Checklist:

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

@paurakhsharma paurakhsharma self-assigned this May 20, 2019
@codecov
Copy link

codecov bot commented May 20, 2019

Codecov Report

Merging #35269 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #35269      +/-   ##
============================================
- Coverage     65.54%   65.53%   -0.01%     
  Complexity    18647    18647              
============================================
  Files          1218     1218              
  Lines         70546    70545       -1     
  Branches       1288     1288              
============================================
- Hits          46236    46234       -2     
- Misses        23933    23934       +1     
  Partials        377      377
Flag Coverage 螖 Complexity 螖
#javascript 53.69% <酶> (酶) 0 <酶> (酶) 猬囷笍
#phpunit 66.9% <酶> (-0.01%) 18647 <酶> (酶)
Impacted Files Coverage 螖 Complexity 螖
...eratedfilesharing/lib/Controller/OcmController.php 66.06% <0%> (-0.21%) 30% <0%> (酶)
lib/private/Server.php 86.57% <0%> (-0.12%) 253% <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 c0dc3cd...2c144f6. Read the comment docs.

@codecov
Copy link

codecov bot commented May 20, 2019 •
edited

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #35269   +/-   ##
=========================================
  Coverage     65.54%   65.54%           
  Complexity    18647    18647           
=========================================
  Files          1218     1218           
  Lines         70546    70546           
  Branches       1288     1288           
=========================================
  Hits          46236    46236           
  Misses        23933    23933           
  Partials        377      377
Flag Coverage 螖 Complexity 螖
#javascript 53.69% <酶> (酶) 0 <酶> (酶) 猬囷笍
#phpunit 66.9% <酶> (酶) 18647 <酶> (酶) 猬囷笍

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 6db604e...20149a7. Read the comment docs.

@paurakhsharma paurakhsharma force-pushed the cli-Test-configKey branch 2 times, most recently from 6e9e1cc to 42a42d7 Compare May 21, 2019 07:48
skshetry
skshetry reviewed May 21, 2019
View reviewed changes
tests/acceptance/features/bootstrap/OccContext.php Outdated
$system_config = $config_list['system'];
$configArray = $system_config[$key];

foreach ($configArray as $innerConfigArray) {
Copy link
Member

Choose a reason for hiding this comment

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

Guiding comments please .....

http://antirez.com/news/124

Copy link
Member Author

Choose a reason for hiding this comment

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

@skshetry Please have a look at the comments.

@paurakhsharma paurakhsharma force-pushed the cli-Test-configKey branch 2 times, most recently from 2c9315d to 3604510 Compare May 21, 2019 11:22
skshetry
skshetry reviewed May 21, 2019
View reviewed changes
Copy link
Member

@skshetry skshetry left a comment

Choose a reason for hiding this comment

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

馃憤

individual-it
individual-it requested changes May 22, 2019
View reviewed changes
tests/acceptance/features/bootstrap/OccContext.php Show resolved Hide resolved
tests/acceptance/features/bootstrap/OccContext.php Outdated
increment the foundCount value
Note: all the values from the tableNode should be present inside the same
config array that's why foundCount is used */
if ($innerConfigArray[$value['key']] === $value['value']) {
Copy link
Member

Choose a reason for hiding this comment

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

we could delete from the table the found keys and then the ones that are not found will be left over, so the error can show which one were not found

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that seems to be the better way.

@individual-it
Copy link
Member

@paurakhsharma please backport and mark in #35207

@paurakhsharma paurakhsharma merged commit 3c1b91c into master May 23, 2019
@delete-merged-branch delete-merged-branch bot deleted the cli-Test-configKey branch May 23, 2019 11:24
@paurakhsharma paurakhsharma mentioned this pull request May 24, 2019
Merged
@paurakhsharma
Copy link
Member Author

Backport on: #35320

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skshetry skshetry skshetry left review comments

@individual-it individual-it individual-it approved these changes

@dpakach dpakach Awaiting requested review from dpakach

@phil-davis phil-davis Awaiting requested review from phil-davis

Assignees

@paurakhsharma paurakhsharma

Labels
3 - To Review dev:acceptance-tests QA:team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@paurakhsharma @individual-it @skshetry @dpakach @phil-davis