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

PHP CS fixer command executed #193

Merged

Conversation

jonesdev
Copy link
Contributor

The native_function_invocation option within PHP CS Fixer was used for this issue.

@ankitpokhrel
Copy link
Owner

@jonesdev Thank you for picking this up.

We are mocking native php functions on unit tests. Tests are failing because a call to unlink($r) can be mocked, but a call to \unlink($r) can't. I suggest you to exclude namespace from the following methods that are mocked during tests until we switch to proper integration tests:

fseek, fread, fwrite, unlink, is_readable, hash_algos, base64_decode, connection_status

@jonesdev
Copy link
Contributor Author

jonesdev commented Oct 2, 2019

@jonesdev Thank you for picking this up.

We are mocking native php functions on unit tests. Tests are failing because a call to unlink($r) can be mocked, but a call to \unlink($r) can't. I suggest you to exclude namespace from the following methods that are mocked during tests until we switch to proper integration tests:

fseek, fread, fwrite, unlink, is_readable, hash_algos, base64_decode, connection_status

Thanks for your advice - I'll try to rectify this later if I get a few minutes.

@jonesdev
Copy link
Contributor Author

jonesdev commented Oct 2, 2019

@ankitpokhrel

I went about it via the following way:

image

And the tests seem to be failing still - am I misunderstanding here?

Failed asserting that exception of type "\TusPhp\Exception\FileException" is thrown.

@ankitpokhrel
Copy link
Owner

@jonesdev you are in a right direction. Check the diff below, it works fine for me. You will need to do similar change in Client.php and Server.php.

--- a/src/File.php
+++ b/src/File.php
@@ -316,7 +316,7 @@ class File
             $this->seek($output, $this->offset);

             while ( ! \feof($input)) {
-                if (CONNECTION_NORMAL !== \connection_status()) {
+                if (CONNECTION_NORMAL !== connection_status()) {
                     throw new ConnectionException('Connection aborted by user.');
                 }

@@ -402,7 +402,7 @@ class File
      */
     public function seek($handle, int $offset, int $whence = SEEK_SET) : int
     {
-        $position = \fseek($handle, $offset, $whence);
+        $position = fseek($handle, $offset, $whence);

         if (-1 === $position) {
             throw new FileException('Cannot move pointer to desired position.');
@@ -423,7 +423,7 @@ class File
      */
     public function read($handle, int $chunkSize) : string
     {
-        $data = \fread($handle, $chunkSize);
+        $data = fread($handle, $chunkSize);

         if (false === $data) {
             throw new FileException('Cannot read file.');
@@ -445,7 +445,7 @@ class File
      */
     public function write($handle, string $data, $length = null) : int
     {
-        $bytesWritten = \is_int($length) ? \fwrite($handle, $data, $length) : \fwrite($handle, $data);
+        $bytesWritten = \is_int($length) ? fwrite($handle, $data, $length) : fwrite($handle, $data);

         if (false === $bytesWritten) {
             throw new FileException('Cannot write to a file.');

@jonesdev
Copy link
Contributor Author

jonesdev commented Oct 3, 2019

@jonesdev you are in a right direction. Check the diff below, it works fine for me. You will need to do similar change in Client.php and Server.php.

--- a/src/File.php
+++ b/src/File.php
@@ -316,7 +316,7 @@ class File
             $this->seek($output, $this->offset);

             while ( ! \feof($input)) {
-                if (CONNECTION_NORMAL !== \connection_status()) {
+                if (CONNECTION_NORMAL !== connection_status()) {
                     throw new ConnectionException('Connection aborted by user.');
                 }

@@ -402,7 +402,7 @@ class File
      */
     public function seek($handle, int $offset, int $whence = SEEK_SET) : int
     {
-        $position = \fseek($handle, $offset, $whence);
+        $position = fseek($handle, $offset, $whence);

         if (-1 === $position) {
             throw new FileException('Cannot move pointer to desired position.');
@@ -423,7 +423,7 @@ class File
      */
     public function read($handle, int $chunkSize) : string
     {
-        $data = \fread($handle, $chunkSize);
+        $data = fread($handle, $chunkSize);

         if (false === $data) {
             throw new FileException('Cannot read file.');
@@ -445,7 +445,7 @@ class File
      */
     public function write($handle, string $data, $length = null) : int
     {
-        $bytesWritten = \is_int($length) ? \fwrite($handle, $data, $length) : \fwrite($handle, $data);
+        $bytesWritten = \is_int($length) ? fwrite($handle, $data, $length) : fwrite($handle, $data);

         if (false === $bytesWritten) {
             throw new FileException('Cannot write to a file.');

Thank you, I'll try to work on this later.

@codecov
Copy link

codecov bot commented Oct 3, 2019

Codecov Report

Merging #193 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##             master   #193   +/-   ##
=======================================
  Coverage       100%   100%           
  Complexity      335    335           
=======================================
  Files            19     19           
  Lines           907    907           
=======================================
  Hits            907    907
Impacted Files Coverage Δ Complexity Δ
src/Config.php 100% <100%> (ø) 8 <0> (ø) ⬇️
src/Request.php 100% <100%> (ø) 27 <0> (ø) ⬇️
src/Response.php 100% <100%> (ø) 12 <0> (ø) ⬇️
src/Middleware/Middleware.php 100% <100%> (ø) 8 <0> (ø) ⬇️
src/Cache/FileStore.php 100% <100%> (ø) 33 <0> (ø) ⬇️
src/Tus/AbstractTus.php 100% <100%> (ø) 8 <0> (ø) ⬇️
src/Cache/RedisStore.php 100% <100%> (ø) 12 <0> (ø) ⬇️
src/Tus/Client.php 100% <100%> (ø) 65 <0> (ø) ⬇️
src/Tus/Server.php 100% <100%> (ø) 91 <0> (ø) ⬇️
src/File.php 100% <100%> (ø) 52 <0> (ø) ⬇️
... and 1 more

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 c09c8e4...bf06f2a. Read the comment docs.

@jonesdev
Copy link
Contributor Author

jonesdev commented Oct 3, 2019

@ankitpokhrel

Output of PHPUnit:

❯ vendor/bin/phpunit
PHPUnit 7.5.16 by Sebastian Bergmann and contributors.

........................SSSSSSSSSSS............................  63 / 219 ( 28%)
............................................................... 126 / 219 ( 57%)
............................................................... 189 / 219 ( 86%)
..............................                                  219 / 219 (100%)

Time: 1.57 seconds, Memory: 20.00 MB

OK, but incomplete, skipped, or risky tests!
Tests: 219, Assertions: 498, Skipped: 11.

@ankitpokhrel
Copy link
Owner

Looks good now. Thanks!

Copy link
Owner

@ankitpokhrel ankitpokhrel left a comment

Choose a reason for hiding this comment

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

Fixes #190

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

Successfully merging this pull request may close these issues.

None yet

2 participants