Slightly confused about coverage related messages about coveragechecker HOT 18 CLOSED

Can you send me the diff files and phpcs files (either upload or email me), the output should look similar to

100.00% Covered

(and have an exit code of 1) if successful

66.67% Covered, Missed lines 
'changedFile.php' has no coverage for the following lines:
Line 3:
The message reported from phpcs

The 66.67% relates to 2 / 3 lines in the pull request / difference being correctly formatted.

0% with no missed lines reported below seems like a bug though

from coveragechecker.

diff.txt

phpcs.txt
(had to change the extension of phpjson.txt) to be able to upload)

But yeah I made a minor per violation and the rest of the output makes sense however the coverage piece just throws me off.

from coveragechecker.

Thanks for getting back to me on this.

from coveragechecker.

I get the following

0.00% Covered, Missed lines 
'app/Http/Controllers/HomeController.php' has no coverage for the following lines:
Line 37:
	No space found after comma in function call
Failing due to coverage being lower than threshold

using the following command
diffFilter --phpcs diff.txt phpcs.txt

(diffFilter is on my path)

So the 0% in this case means that every line contains a violation (there is only one line)
Then app/Http/Controllers/HomeController.php line 37 failed due to "No space found after comma in function call"

Do you get the additional lines ?

I will add some documentation about the percentages to make them more clear

from coveragechecker.

My whole file:

<?php

namespace App\Http\Controllers;

use App\Http\Requests;
use App\County;
use Carbon\Carbon;
use App\Http\Requests\ContactFormRequest;
use Illuminate\Support\Facades\Mail;
use App\Mail\ContactFormSubmitted;
use App\User;

class HomeController extends Controller
{
    /**
     * Create a new controller instance.
     *
     * @return void
     */
    public function __construct()
    {
    }

    /**
     * Show the home page.
     *
     * @return \Illuminate\Http\Response
     */
    public function index()
    {
        $filter = array(
            ['name', '=', ucfirst('harris')],
            ['state', '=', strtoupper('tx')]
        );

        $county = County::where($filter)->first();
        return view('prelogin/home',compact('county'));
    }

    public function createContactForm()
    {
        return view('contact');
    }

    public function storeContactForm(ContactFormRequest $request)
    {
        $name = $request->input('name');
        $email = $request->input('email');
        $bodyMessage = $request->input('bodyMessage');

        Mail::to(User::where('email', '[email protected]') -> first())->send(new ContactFormSubmitted($name, $email, $bodyMessage));
        return \Redirect::route('contact')
            ->with('flash_notification.message', 'Thanks! We will get back to you shortly');
    }
}
`

Just a controller Im using to test this. 

I then made a couple of additional PSR violations to the file above and now I get:


`
Failing due to coverage being lower than threshold
array:1 [
  "app/Http/Controllers/HomeController.php" => array:6 [
    0 => "20.00% Covered, Missed lines "
    1 => "'app/Http/Controllers/HomeController.php' has no coverage for the following lines:"
    2 => "Line 37:"
    3 => "\tNo space found after comma in function call"
    5 => "Line 47:"
    6 => "\tExpected 0 spaces before closing bracket; 1 found"
  ]
]
`

perhaps this is just my lack of understanding. 

from coveragechecker.

Not sure what's wrapping the error in array syntax ? Stderr looks fine (that is the output which says failing due to )

Stdout has all lines wrapped in an array which makes the output much more difficult to read.

Is this running inside another tool ?

Edit.
Is this running inside an artisan command ? I think artisan::output displays similar

from coveragechecker.

foreach ($changedFilesList as $filePath) {
            //run lint on entire file
            $lintCommand = sprintf("cd %s &&  php vendor/bin/phpcs --standard=PSR2 --encoding=utf-8 --report=json %s > phpcs.json", $this->basePath, $filePath);
			$lintOutput = `$lintCommand`;

            //create the diff
            $diffCommand = sprintf("cd %s && git diff --cached %s > diff.txt", $this->basePath, $filePath);
            $diffOutput = `$diffCommand`;


            $checkerCommand = "php vendor/bin/diffFilter --phpcs diff.txt phpcs.json";
            $checkerCommand = `$checkerCommand`;

            if ($checkerCommand){

                $resultScan[$filePath] = array_filter(explode("\n", $checkerCommand), function($item) { return strlen($item) > 0; });
            }

		}

from coveragechecker.

Sorry I should have mentioned earlier. Trying just to get failures / warnings in an array . Ran it with 0 at the end to not throw exception

from coveragechecker.

I will make tye output optionally in a easier to parse format.

Should be sorted by tomorrow

Thanks for your feedback :)

from coveragechecker.

many thanks in advance for the quick response.

from coveragechecker.

php supports a file list

git diff --name-only --diff-filter=ACMR -- '*.php'

Will list all changed files (the filter is to not list removed files)

you can then use

phpcs --file-list=
and point that that the file generated from the diff command, only the changed files will then be run

Your build script would be

#!/bin/bash -e

git diff --cached  --name-only --diff-filter=ACMR -- '*.php' >  changedFiles.txt
git diff --cached > diff.txt

php vendor/bin/phpcs --standard=PSR2 --encoding=utf-8 --report=json --filelist=changedFiles.txt > phpcs.json || true

php vendor/bin/diffFilter --phpcs diff.txt phpcs.json

calling build.sh before you commit (maybe on a hook ?) would then enforce the changes

Am currently working on a format for the output

currently looks like

{
    coverage: 33.3,
    status: "Failed",
    violations: [
           file: [
                 {lineNumber: 30, message: "The error"},
            ]
     ]
}

Would that be ok or would you need any more information ?

from coveragechecker.

very nice. I actually used a very similar implementation. I think once you modify the output it would get me going. One minor suggestion, on your output above if it could also list if it is a warning or an error that would be great. So for example:

{
    coverage: 33.3,
    status: "Failed",
    violations: [
           file: [
                 {lineNumber: 30, message: "The error", 'warning'},
                 {lineNumber: 35, message: "The error", 'error'},
            ]
     ]
}

This is a highly useful library. Kudos.

from coveragechecker.

Contains two PR's (but I needed one part of the other one to make this work)

#18

if you could try that branch (addOutputFilters) and let me know what you think, I have moved everything to stderror unless its the real output (json in this case) so you should be able to just read from stdout.

It is called by adding --report=json on the command line eg

diffFilter --phpcs --report=json diff.txt phpcs.json

from coveragechecker.

At work now but I will check it out first thing afterwards. Thanks Scott

from coveragechecker.

No problem

The warning / error distinction I have not added.

--phpcs only reports on errors.

--phpcsStrict will report errors and warnings. These can be changed from the phpcs config also

Thanks for taking the time to try this out :)

from coveragechecker.

Hey thanks for the update. So I did get a chance to test the branch. The output looks sweet. However I think there may be a bug. When you just make a change to a PHP file that is not a PSR violation, it returns:

class stdClass#2 (3) {
public $coverage =>
string(6) "100.00"
public $status =>
string(6) "Failed"
public $violations =>
array(0) {
}
}

I think that needs to be a success. The JSON output makes it so much easier. PS I sent you mail also.

Thanks Scott

from coveragechecker.

Thanks for that, Found I accidentally flipped the condition. Should be resolved now.

from coveragechecker.

Had some extra time to test, This has been merged and tagged as 0.7

from coveragechecker.