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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

files external unit tests broken #13754

Closed
MorrisJobke opened this issue Jan 29, 2015 · 12 comments
Closed

files external unit tests broken #13754

MorrisJobke opened this issue Jan 29, 2015 · 12 comments
Assignees
@PVince81
Labels
3 - To Review app:files_external regression Type:Bug
Milestone
8.0

Comments

@MorrisJobke
Copy link
Contributor

Due to the move to exceptions instead of true/false as return value:


1) Test\Files\Storage\DAV::testDirectories with data set #0 ('folder')
OCP\Files\StorageNotAvailableException: Sabre\DAV\Exception\MethodNotAllowed: Method not allowed

/var/jenkins/workspace/server-master-linux-externals/database/sqlite/external/webdav-ownCloud/label/vm-slave-02/lib/private/files/storage/dav.php:572
/var/jenkins/workspace/server-master-linux-externals/database/sqlite/external/webdav-ownCloud/label/vm-slave-02/lib/private/files/storage/dav.php:444
/var/jenkins/workspace/server-master-linux-externals/database/sqlite/external/webdav-ownCloud/label/vm-slave-02/lib/private/files/storage/dav.php:101
/var/jenkins/workspace/server-master-linux-externals/database/sqlite/external/webdav-ownCloud/label/vm-slave-02/tests/lib/files/storage/storage.php:88

2) Test\Files\Storage\DAV::testDirectories with data set #1 (' folder')
OCP\Files\StorageNotAvailableException: Sabre\DAV\Exception\MethodNotAllowed: Method not allowed

/var/jenkins/workspace/server-master-linux-externals/database/sqlite/external/webdav-ownCloud/label/vm-slave-02/lib/private/files/storage/dav.php:572
/var/jenkins/workspace/server-master-linux-externals/database/sqlite/external/webdav-ownCloud/label/vm-slave-02/lib/private/files/storage/dav.php:444
/var/jenkins/workspace/server-master-linux-externals/database/sqlite/external/webdav-ownCloud/label/vm-slave-02/lib/private/files/storage/dav.php:101
/var/jenkins/workspace/server-master-linux-externals/database/sqlite/external/webdav-ownCloud/label/vm-slave-02/tests/lib/files/storage/storage.php:88

3) Test\Files\Storage\DAV::testDirectories with data set #2 ('folder ')
OCP\Files\StorageNotAvailableException: Sabre\DAV\Exception\MethodNotAllowed: Method not allowed

/var/jenkins/workspace/server-master-linux-externals/database/sqlite/external/webdav-ownCloud/label/vm-slave-02/lib/private/files/storage/dav.php:572
/var/jenkins/workspace/server-master-linux-externals/database/sqlite/external/webdav-ownCloud/label/vm-slave-02/lib/private/files/storage/dav.php:444
/var/jenkins/workspace/server-master-linux-externals/database/sqlite/external/webdav-ownCloud/label/vm-slave-02/lib/private/files/storage/dav.php:101
/var/jenkins/workspace/server-master-linux-externals/database/sqlite/external/webdav-ownCloud/label/vm-slave-02/tests/lib/files/storage/storage.php:88

4) Test\Files\Storage\DAV::testDirectories with data set #3 ('folder with space')
OCP\Files\StorageNotAvailableException: Sabre\DAV\Exception\MethodNotAllowed: Method not allowed

/var/jenkins/workspace/server-master-linux-externals/database/sqlite/external/webdav-ownCloud/label/vm-slave-02/lib/private/files/storage/dav.php:572
/var/jenkins/workspace/server-master-linux-externals/database/sqlite/external/webdav-ownCloud/label/vm-slave-02/lib/private/files/storage/dav.php:444
/var/jenkins/workspace/server-master-linux-externals/database/sqlite/external/webdav-ownCloud/label/vm-slave-02/lib/private/files/storage/dav.php:101
/var/jenkins/workspace/server-master-linux-externals/database/sqlite/external/webdav-ownCloud/label/vm-slave-02/tests/lib/files/storage/storage.php:88

5) Test\Files\Storage\DAV::testDirectories with data set #4 ('spéciäl földer')
OCP\Files\StorageNotAvailableException: Sabre\DAV\Exception\MethodNotAllowed: Method not allowed

/var/jenkins/workspace/server-master-linux-externals/database/sqlite/external/webdav-ownCloud/label/vm-slave-02/lib/private/files/storage/dav.php:572
/var/jenkins/workspace/server-master-linux-externals/database/sqlite/external/webdav-ownCloud/label/vm-slave-02/lib/private/files/storage/dav.php:444
/var/jenkins/workspace/server-master-linux-externals/database/sqlite/external/webdav-ownCloud/label/vm-slave-02/lib/private/files/storage/dav.php:101
/var/jenkins/workspace/server-master-linux-externals/database/sqlite/external/webdav-ownCloud/label/vm-slave-02/tests/lib/files/storage/storage.php:88

6) Test\Files\Storage\DAV::testMimeType
OCP\Files\StorageNotAvailableException: Sabre\DAV\Exception\NotFound: Resource not found.

/var/jenkins/workspace/server-master-linux-externals/database/sqlite/external/webdav-ownCloud/label/vm-slave-02/lib/private/files/storage/dav.php:572
/var/jenkins/workspace/server-master-linux-externals/database/sqlite/external/webdav-ownCloud/label/vm-slave-02/lib/private/files/storage/dav.php:402
/var/jenkins/workspace/server-master-linux-externals/database/sqlite/external/webdav-ownCloud/label/vm-slave-02/tests/lib/files/storage/storage.php:149

cc @icewind1991 @PVince81 Can you fix that? I simply just can fix the tests regarding what is currently implemented and not what is actually wanted. So it's maybe better if you fix the unit tests ;) Thanks

If you have docker installed you can simply run ./autotest-external.sh sqlite otherwise add the files_external app to the enable_apps-php and run the ./autotest.sh sqlite PATHTOFILE ;)
@MorrisJobke MorrisJobke added this to the 8.0-current milestone Jan 29, 2015
@DeepDiver1975
Copy link
Member

started to break in https://ci.owncloud.org/job/server-master-linux-externals/498/

@MorrisJobke
Copy link
Contributor Author

@DeepDiver1975 404 :(

@PVince81
Copy link
Contributor

@MorrisJobke ./../apps/files_external/tests/env/start-webdav-ownCloud.sh: line 29: replace: command not found

@PVince81
Copy link
Contributor

@MorrisJobke

1) Test\Files\Storage\DAV::testRoot
include(files_external/tests/config.webdav.php): failed to open stream: No such file or directory

/srv/www/htdocs/owncloud/apps/files_external/tests/backends/webdav.php:19
/srv/www/htdocs/owncloud/apps/files_external/tests/backends/webdav.php:19

Isn't the setup fully automatic or I need to manually edit some configs ?

@PVince81
Copy link
Contributor

Looking at the error message: we should probably rethrow "MethodNotAllowed" or return false... because "MethodNotAllowed" does not mean that the storage is not available.

@PVince81
Copy link
Contributor

and it seems in some cases the exception is "NotFound" which might be expected. Need to catch that too.

@PVince81 PVince81 self-assigned this Jan 30, 2015
@PVince81
Copy link
Contributor

What a mess... we shouldn't have introduced exceptions that early, or not catch it this way.
The problem is that other storages will not throw exceptions, but that "storage.php" unit test applies for all. We probably don't want to set different expectations for different storages right now, until we move everything to using exceptions.

Still trying to continue patching...

@PVince81 PVince81 mentioned this issue Jan 30, 2015
Merged
@PVince81
Copy link
Contributor

Fix is here: #13791

@RobinMcCorkell
Copy link
Member

(not specific to this issue in particular, just writing down my thoughts)

The thing I don't like about exception handling in storages is that code needs to be massively duplicated - every storage method needs to handle exceptions that may be thrown by 3rd party code, then either rethrow a useful exception or return false. I wish there was some way we could avoid having hundreds of try{} catch{} blocks...

@PVince81
Copy link
Contributor

I think in other software usually they would have their own exceptions: Storage\NotFound, Storage\Forbidden, Storage\MethodNotAllowed and they would wrap the 3rd party code's exception into their own and rethrow. That would still require catching the exceptions everywhere or using something like that convertSabreException.
But like you I'm also wondering how to avoid having so many try/catch blocks.

Let's continue the discussion in this ticket: #13792

@MorrisJobke
Copy link
Contributor Author

Fix is in #13791

@DeepDiver1975
Copy link
Member

Fix is in #13791

merged -> close

@lock lock bot locked as resolved and limited conversation to collaborators Aug 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@PVince81 PVince81

Labels
3 - To Review app:files_external regression Type:Bug
Projects
None yet
Milestone
8.0
Development

No branches or pull requests
4 participants
@MorrisJobke @PVince81 @DeepDiver1975 @RobinMcCorkell