Improve behavior for single-file plugins

[mukeshpanchal27]

Fixes #307

This PR improve the behaviour for the single file plugins. You can test single file plugin such as Hello Dolly.

CLI command: npm run wp-env run cli -- wp plugin check hello.php

[mukeshpanchal27]

In an attempt to test the Plugin_Request_Utility::is_single_file_plugin() method, I tried to add a unit test in the plugins folder (/tests/phpunit/testdata/plugins/foo-bar.php). However, due to the condition at this line in Plugin_Context, it appears that this approach will not work for unit tests.

The issue might be related to the behaviour of Plugin_Context::location(). Whether this is a bug that needs to be addressed or an accepted behaviour would require further investigation.

public function test_single_file_plugin_without_error() {
	$check_context = new Check_Context( UNIT_TESTS_PLUGIN_DIR . 'foo-bar.php' );
	$checks        = new Checks();
	$checks_to_run = array(
		new Plugin_Readme_Check(),
		new Trademarks_Check(),
	);

	add_filter(
		'wp_plugin_check_checks',
		function ( $checks ) {
			return array(
				'plugin_readme' => new Plugin_Readme_Check(),
				'trademarks'    => new Trademarks_Check(),
			);
		}
	);

	$results = $checks->run_checks( $check_context, $checks_to_run );

	$this->assertInstanceOf( Check_Result::class, $results );

	$this->assertEmpty( $results->get_warnings() );
	$this->assertEmpty( $results->get_errors() );
}

[felixarntz]

@mukeshpanchal27 Good catch, and I think you're on the right track with the approach. The solution could be simplified though, some of the changes aren't needed.

[swissspidy]

Nice and concise!

[felixarntz]

Thanks @mukeshpanchal27, looks great!

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (db28ffe) 39.49% compared to head (5da0075) 36.57%.

Additional details and impacted files

@@            Coverage Diff             @@
##            trunk     #319      +/-   ##
==========================================
- Coverage   39.49%   36.57%   -2.92%     
==========================================
  Files          43       43              
  Lines        2028     2034       +6     
==========================================
- Hits          801      744      -57     
- Misses       1227     1290      +63     

Flag	Coverage Δ
feature	36.57% <100.00%> (-2.92%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[felixarntz]

@swissspidy I believe the codecov report failing here was added as part of the Behat tests PR? Can you advise how to best proceed with this? How would we fix this, and is it even relevant here?

[swissspidy]

@felixarntz The failed Behat check was just a flake, can be simply restarted. Or do you mean the codecov/project status check? That's purely informative. If we want to make it blocking, we can though.

[felixarntz]

@swissspidy What's the threshold for codecov to fail? Making it blocking is probably a good idea to make sure new tests are added for new code.

On that note @mukeshpanchal27, could you maybe add 2 unit tests for the extra condition in Plugin_Readme_Check and Trademarks_Check, using single file plugins?

[swissspidy]

Not sure what the default threshold is, I suppose any negative change causes a failed check.

Note that right now coverage right now is only collected through Behat tests, not the unit tests. That needs to be added separately. Happy to whip up a quick PR.

[felixarntz]

@swissspidy Oh okay. In that case it would be great if you could open an extra PR for this, or feel free to use this one.

[swissspidy]

Working on a new one over here: #321

[mukeshpanchal27]

On that note @mukeshpanchal27, could you maybe add 2 unit tests for the extra condition in Plugin_Readme_Check and Trademarks_Check, using single file plugins?

@felixarntz I have added two tests, but Codecov is indicating an error. Could we address the coverage issue once this pull request is merged?

[felixarntz]

Thank you @mukeshpanchal27 for adding the two tests, those looks great.

Regarding the continuing workflow error, I think we'll just have to wait for #321 to be merged, then we can refresh this PR with trunk and then it should hopefully pass.

[swissspidy]

Why not just merge this now? 🤔

[felixarntz]

@swissspidy I think we should avoid failing tests. There's no rush on getting this particular PR shipped, so I don't think we have to merge this before addressing the failures.

[swissspidy]

To be fair it's not a failed test, but a failed status check because there is more code being introduced and we don't have code coverage set up.