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.

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

[5.2] Add Filesystem tests. #12940

Merged
merged 1 commit into from
Mar 31, 2016
Merged

[5.2] Add Filesystem tests. #12940

merged 1 commit into from
Mar 31, 2016

Conversation

bancur
Copy link
Contributor

@bancur bancur commented Mar 31, 2016

Added 3 tests for previously untested Filesystems requireOnce, copy and isFile methods.

@@ -324,4 +324,40 @@ public function testSharedGet()
$this->assertTrue($result === 1);
@unlink(__DIR__.'/file.txt');
}

public function testRequireOnceRequiresFileProperly()
Copy link
Member

Choose a reason for hiding this comment

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

This test will break if you run phpunit with multiple runs

Copy link
Member

Choose a reason for hiding this comment

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

Or rather, it won't really test anything properly.

Copy link
Member

Choose a reason for hiding this comment

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

You should call requireOnce more than one to check it only requires the file once.

@taylorotwell taylorotwell merged commit f0ad164 into laravel:5.2 Mar 31, 2016
@GrahamCampbell GrahamCampbell changed the title Add Filesystem tests. [5.2] Add Filesystem tests. Mar 31, 2016
file_put_contents(__DIR__.'/foo/foo.txt', 'contents');
$this->assertTrue($filesystem->isFile(__DIR__.'/foo/foo.txt'));
$this->assertFalse($filesystem->isFile(__DIR__.'./foo'));
@unlink('/foo/foo.txt');
Copy link
Contributor

Choose a reason for hiding this comment

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

This line should be:

@unlink(__DIR__ . '/foo/foo.txt');

Currently, this leaves tests/Filesystem/foo/foo.txt lying around and this breaks several tests if the test suite is run a second time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants