X Tutup
Skip to content

fix: Update StoragesBackendService.php to default handling#41273

Merged
jnweiger merged 2 commits intomasterfrom
jw-more-fixes-for-41271
Jun 24, 2024
Merged

fix: Update StoragesBackendService.php to default handling#41273
jnweiger merged 2 commits intomasterfrom
jw-more-fixes-for-41271

Conversation

@jnweiger
Copy link
Contributor

@jnweiger jnweiger commented Jun 21, 2024 •
edited
Loading

fix: another occurance of #41272

@jnweiger jnweiger requested review from DeepDiver1975 and pako81 June 21, 2024 14:22
@pako81
Copy link

pako81 commented Jun 21, 2024

category is missing in commit message

@jnweiger jnweiger changed the title Update StoragesBackendService.php to default handling fix: Update StoragesBackendService.php to default handling Jun 21, 2024
@phil-davis
Copy link
Contributor

phil-davis commented Jun 21, 2024

The commit needs to have the commit message edited, and then force-push the change.

@jnweiger jnweiger force-pushed the jw-more-fixes-for-41271 branch from d35714d to 8af82c2 Compare June 21, 2024 14:50
@jnweiger
Copy link
Contributor Author

jnweiger commented Jun 21, 2024

The commit needs to have the commit message edited, and then force-push the change.

cannot be done through web UI? sigh.
second attempt... done.

@owncloud owncloud deleted a comment from update-docs bot Jun 21, 2024
@phil-davis
Copy link
Contributor

phil-davis commented Jun 21, 2024

cannot be done through web UI?

You can edit the PR title, but that is not editing the commit message of the commit(s).

@jnweiger
Copy link
Contributor Author

jnweiger commented Jun 24, 2024

@phil-davis I still struggle to understand how to make drone happy.
This error occurs in php unit tests, but seems unrelated to my one-liner fix:

There was 1 failure:

  1. Test\Files\External\StoragesBackendServiceTest::testUserMountingBackends

OCP\Files\External\Backend\Backend::removeVisibility(1) was not expected to be called.

@phil-davis
Copy link
Contributor

phil-davis commented Jun 24, 2024 

$ make test-php-unit NOCOVERAGE=true TEST_PHP_SUITE=tests/lib/Files/External/StoragesBackendServiceTest.php
...
PHPUNIT="/home/phil/git/owncloud/core/lib/composer/phpunit/phpunit/phpunit" build/autotest.sh sqlite tests/lib/Files/External/StoragesBackendServiceTest.php
Using PHP executable /usr/bin/php
Parsing all files in lib/public for the presence of @since or @deprecated on each method...

Using database oc_autotest
Setup environment for sqlite testing on local storage ...
Updated 0 paths from the index
Installing ....
creating sqlite db
ownCloud was successfully installed
Testing with sqlite ...
No coverage
/home/phil/git/owncloud/core/lib/composer/phpunit/phpunit/phpunit --configuration phpunit-autotest.xml --log-junit autotest-results-sqlite.xml ../tests/lib/Files/External/StoragesBackendServiceTest.php 
PHPUnit 9.6.19 by Sebastian Bergmann and contributors.

Runtime:       PHP 7.4.33
Configuration: phpunit-autotest.xml

....F.                                                              6 / 6 (100%)

Time: 00:00.023, Memory: 20.00 MB

There was 1 failure:

1) Test\Files\External\StoragesBackendServiceTest::testUserMountingBackends
OCP\Files\External\Backend\Backend::removeVisibility(1) was not expected to be called.

/home/phil/git/owncloud/core/lib/private/Files/External/StoragesBackendService.php:124
/home/phil/git/owncloud/core/tests/lib/Files/External/StoragesBackendServiceTest.php:177

FAILURES!
Tests: 6, Assertions: 25, Failures: 1.
make: *** [Makefile:194: test-php-unit] Error 1

I get this unit test fail locally with your branch but not with master

I will have a look at what happens differently in the code.

@phil-davis
Copy link
Contributor

phil-davis commented Jun 24, 2024

Should be fixed - now the call to getAppValue is done with getAppValue('files_external', 'allow_user_mounting', 'no')
So we need to put the 'no' into the test returnValueMap and then the test mocking will recognize the call and return the needed value.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 24, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@jnweiger jnweiger merged commit c40a37d into master Jun 24, 2024
@phil-davis phil-davis deleted the jw-more-fixes-for-41271 branch June 24, 2024 12:13
phil-davis added a commit that referenced this pull request Jun 25, 2024
@jnweiger @phil-davis
fix: Update StoragesBackendService.php to default handling (#41273)
497c194 
* fix: Update StoragesBackendService.php to default handling

also needed for #41271

* fix: Update StoragesBackendServiceTest to pass

---------

Co-authored-by: Phil Davis <phil@jankaritech.com>
@phil-davis phil-davis mentioned this pull request Jun 25, 2024
Merged
11 tasks
phil-davis added a commit that referenced this pull request Jul 5, 2024
@phil-davis
Merge pull request #41275 from owncloud/backport-20240625
792888b 
[10.15.0] fix: Update StoragesBackendService.php to default handling (#41273)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@phil-davis phil-davis phil-davis approved these changes

@DeepDiver1975 DeepDiver1975 Awaiting requested review from DeepDiver1975

+1 more reviewer

@pako81 pako81 pako81 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jnweiger @pako81 @phil-davis
X Tutup