Skip to content

#27854 non existing folder skeletondirectory#29250

Closed
Gandolf90 wants to merge 9 commits intoowncloud:masterfrom
Gandolf90:27854-non-existing-folder-skeletondirectory
Closed

#27854 non existing folder skeletondirectory#29250
Gandolf90 wants to merge 9 commits intoowncloud:masterfrom
Gandolf90:27854-non-existing-folder-skeletondirectory

Conversation

@Gandolf90
Copy link
Contributor

@Gandolf90 Gandolf90 commented Oct 15, 2017

Description

Related Issue

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)

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.

adding throw exception if config false
adding new testcase to check if config has incorrekt skeletondirectory
incorrect test-name
@CLAassistant
Copy link

CLAassistant commented Oct 15, 2017

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@DeepDiver1975 DeepDiver1975 left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your contribution!
Very welcome!
Please make sure to squash our commits. THX

$skeletonDirectory = \OCP\Config::getSystemValue('skeletondirectory', \OC::$SERVERROOT . '/core/skeleton');

if(!is_dir($skeletonDirectory)) {
throw new OC\HintException('Can not Access to Skeleton Folder, check for right Path in config.php');
Copy link
Member

Choose a reason for hiding this comment

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

  • please add the value of $skeletonDirectory
  • test should read as
throw new OC\HintException("The skeleton folder '$skeletonFolder' is not accessible.');

}

public function testCopySkeletonDirectory() {
$this->expectException(HintException::class);
Copy link
Member

Choose a reason for hiding this comment

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

please use phpunit annotation - see the phpunit docs

@DeepDiver1975
Copy link
Member

Please also sign the CLA - see the comment above

added new output
added test annotation & check for output
@IljaN
Copy link
Contributor

IljaN commented Oct 18, 2017

Please reference pull requests to issues like this #27854

public function testCopySkeletonDirectory() {
$notExistingDirectory = '/not/existing/Directory';

$this->expectException(HintException::class);
Copy link
Contributor

Choose a reason for hiding this comment

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


$skeletonDirectory = \OCP\Config::getSystemValue('skeletondirectory', \OC::$SERVERROOT . '/core/skeleton');

if(!is_dir($skeletonDirectory)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing space between if and opening parenthesis => if ( )

Missing space between if and opening parenthesis
annotation phpunit fixed
@Gandolf90 Gandolf90 changed the title 27854 non existing folder skeletondirectory #27854 non existing folder skeletondirectory Oct 22, 2017
@IljaN
Copy link
Contributor

IljaN commented Oct 24, 2017

@Gandolf90 Some tests are failing, please fix. See Jenkins for details.

removed use tag and added path to Exception Class
 added user Folde
mocked userfolder
@Gandolf90
Copy link
Contributor Author

@IljaN how can I test the failed Selenium/Travis tests?

@IljaN
Copy link
Contributor

IljaN commented Oct 25, 2017

@Gandolf90 "make test"

added code style space
@Gandolf90
Copy link
Contributor Author

Gandolf90 commented Oct 25, 2017

@IljaN testet local with "make test" and jenkins tests passed
don't know why travis make with php 5.6 settings and selenium error to connect with curl as user

@IljaN
Copy link
Contributor

IljaN commented Oct 25, 2017

@Gandolf90 Don`t worry our CI is like a diva sometimes :)

@phil-davis
Copy link
Contributor

@Gandolf90 Selenium/Travis fails because this is a PR from a forked copy of the repo. So Travis does not provide the job with the authentication keys to the ownCloud SauceLabs account. There are 2 options:

  1. you can ask for access to the ownCloud core repo (which I got as a new community member a few years ago without any hassle) - then clone that and make your own branch in there.
  2. create yourself a SauceLabs open-source free account, then put the details in your own Travis linked to GitHub. Then make a PR of the code in your fork to the master of your fork - Travis and SauceLabs will happily talk using all your own authentication data.

@Gandolf90 Gandolf90 closed this Oct 25, 2017
@Gandolf90
Copy link
Contributor Author

@IljaN or @DeepDiver1975 can you create a temporary access to make the pull request ?

@Gandolf90
Copy link
Contributor Author

@phil-davis thanks for Hint

@IljaN
Copy link
Contributor

IljaN commented Oct 26, 2017

@Gandolf90 Why close?

@Gandolf90
Copy link
Contributor Author

@IljaN Because i wanted to make a Branch from Core Repo and make there the pr as @phil-davis wrote

@phil-davis
Copy link
Contributor

@Gandolf90 when you make the PR in core, please mention it here so it is easy to find.

@Gandolf90
Copy link
Contributor Author

@phil-davis ok

@Gandolf90
Copy link
Contributor Author

@IljaN @DeepDiver1975 @phil-davis here it goes : #29367

@Gandolf90 Gandolf90 deleted the 27854-non-existing-folder-skeletondirectory branch October 28, 2017 09:00
@lock
Copy link

lock bot commented Aug 2, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 2, 2019
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.

6 participants