jenkins-bot has submitted this change and it was merged.
Change subject: Assert fixers do as intended
......................................................................
Assert fixers do as intended
We didn't have any tests that fixers do what we expect them to do. Patch
adds a test for any .php file that has a file of the same name with
.fixed as a suffix to assert the output of the code matches the recorded
fixed content. If no .fixed file exists we assert that the output of
fixing the file is the source file. I have not manually reviewed these
fixed files, they match the current state of the repository and should
perhaps be manually reviewed. This was prompted by I3dca4c37 which
tries to fix a bad fixer that was eating code.
This brings in an additional vfs dependency to allow the test to run
without changing anything on the filesystem, but could probably be
rewritten without it if people feel strongly against the dependency.
.fixed files generated with:
vendor/bin/phpcbf --standard=MediaWiki --suffix=.fixed MediaWiki\Tests\files
Change-Id: Ie7102faf842f56e6b93b04f02e90606f19fcaee1
---
M MediaWiki/Tests/MediaWikiStandardTest.php
A MediaWiki/Tests/MediaWikiTestHelper.php
A MediaWiki/Tests/files/AlternativeSyntax/alternative_syntax_fail.php.fixed
A MediaWiki/Tests/files/ControlStructures/if_else_structure_fail.php.fixed
A
MediaWiki/Tests/files/ExtraCharacters/extra_characters_before_phpopen_tag_fail.php.fixed
A MediaWiki/Tests/files/NamingConventions/case_global_name_fail.php.fixed
A MediaWiki/Tests/files/NamingConventions/wg_global_name2_fail.php.fixed
A MediaWiki/Tests/files/NamingConventions/wg_global_name_fail.php.fixed
A MediaWiki/Tests/files/Usage/dir_usage_fail.php.fixed
A MediaWiki/Tests/files/VariableAnalysis/unused_global_variables_fail.php.fixed
A MediaWiki/Tests/files/WhiteSpace/multiple_empty_lines_fail.php.fixed
A MediaWiki/Tests/files/WhiteSpace/short_array_fail.php.fixed
A
MediaWiki/Tests/files/WhiteSpace/space_after_delim_singleline_comment_fail.php.fixed
A MediaWiki/Tests/files/WhiteSpace/space_before_parens_fail.php.fixed
A MediaWiki/Tests/files/WhiteSpace/spacey_parenthesis_fail.php.fixed
M composer.json
16 files changed, 211 insertions(+), 2 deletions(-)
Approvals:
Legoktm: Looks good to me, approved
jenkins-bot: Verified
diff --git a/MediaWiki/Tests/MediaWikiStandardTest.php
b/MediaWiki/Tests/MediaWikiStandardTest.php
index 44f57a3..72a94a9 100644
--- a/MediaWiki/Tests/MediaWikiStandardTest.php
+++ b/MediaWiki/Tests/MediaWikiStandardTest.php
@@ -33,7 +33,8 @@
public function setUp() {
parent::setUp();
if ( empty( $this->helper ) ) {
- $this->helper = new TestHelper();
+ include_once __DIR__ . '/MediaWikiTestHelper.php';
+ $this->helper = new MediaWikiTestHelper();
}
}
@@ -82,6 +83,30 @@
$this->assertEquals( $expect, $outputStr );
}
+ public static function testFixProvider() {
+ $tests = self::testProvider();
+ foreach ( array_keys( $tests ) as $idx ) {
+ $fixed = $tests[$idx][0] . ".fixed";
+ if ( file_exists( $fixed ) ) {
+ $tests[$idx][2] = $fixed;
+ } else {
+ // no fixes should be applied, assert fixed
+ // file matches original
+ $tests[$idx][2] = $tests[$idx][0];
+ }
+ }
+ return $tests;
+ }
+
+ /**
+ * @dataProvider testFixProvider
+ */
+ public function testFix( $file, $standard, $fixedFile ) {
+ $outputStr = $this->helper->runPhpCbf( $file, $standard );
+ $expect = file_get_contents( $fixedFile );
+ $this->assertEquals( $expect, $outputStr );
+ }
+
/**
* strip down the output to only the warnings
*
diff --git a/MediaWiki/Tests/MediaWikiTestHelper.php
b/MediaWiki/Tests/MediaWikiTestHelper.php
new file mode 100644
index 0000000..38f1670
--- /dev/null
+++ b/MediaWiki/Tests/MediaWikiTestHelper.php
@@ -0,0 +1,67 @@
+<?php
+
+use org\bovigo\vfs\vfsStream;
+
+class MediaWikiTestHelper extends TestHelper {
+ public function __construct() {
+ parent::__construct();
+ $this->vfsRoot = vfsStream::setup( 'root' );
+ }
+
+ public function runPhpCbf( $file, $standard = '' ) {
+ if ( empty( $standard ) ) {
+ $standard = $this->rootDir . '/ruleset.xml';
+ }
+ $defaults = $this->phpcs->getDefaults();
+
+ if (
+ defined( 'PHP_CodeSniffer::VERSION' ) &&
+ version_compare( PHP_CodeSniffer::VERSION, '1.5.0' ) !=
-1
+ ) {
+ $standard = [ $standard ];
+ }
+ $options = [
+ 'encoding' => 'utf-8',
+ 'files' => [ $file ],
+ 'standard' => $standard,
+ 'reports' => [ 'diff' => vfsStream::url(
'root/phpcbf-fixed.diff' ) ]
+ ] + $defaults;
+
+ ob_start();
+ $this->phpcs->process( $options );
+ ob_end_clean();
+
+ if ( !$this->vfsRoot->hasChild( 'phpcbf-fixed.diff' ) ) {
+ // no diff generated, return source file
+ return file_get_contents( $file );
+ }
+
+ $diff = $this->vfsRoot->getChild( 'phpcbf-fixed.diff'
)->getContent();
+ if ( empty( trim( $diff ) ) ) {
+ return file_get_contents( $file );
+ }
+
+ // patch the source file and output to stdout
+ $cmd = "patch -p0 -u -o -";
+ $descriptorSpec = [
+ 0 => [ 'pipe', 'r' ],
+ 1 => [ 'pipe', 'w' ],
+ 2 => [ 'file', '/dev/null', 'w' ],
+ ];
+ $process = proc_open( $cmd, $descriptorSpec, $pipes );
+ if ( !$process ) {
+ throw new RuntimeException( "Failed to run $cmd" );
+ }
+
+ fwrite( $pipes[0], $diff );
+ fclose( $pipes[0] );
+
+ $output = stream_get_contents( $pipes[1] );
+ fclose( $pipes[1] );
+
+ $retval = proc_close( $process );
+
+ // test retval?
+ return $output;
+ }
+}
diff --git
a/MediaWiki/Tests/files/AlternativeSyntax/alternative_syntax_fail.php.fixed
b/MediaWiki/Tests/files/AlternativeSyntax/alternative_syntax_fail.php.fixed
new file mode 100644
index 0000000..94378f9
--- /dev/null
+++ b/MediaWiki/Tests/files/AlternativeSyntax/alternative_syntax_fail.php.fixed
@@ -0,0 +1,29 @@
+<?php
+
+declare ( ticks = 1 ):
+ echo "foo";
+enddeclare;
+
+for ( $i = 0; $i < 5; $i++ ):
+ echo $i;
+endfor;
+
+foreach ( [ 1, 2, 3 ] as $i ):
+ echo $i;
+endforeach;
+
+$x = 1;
+
+if ( $x < 2 ):
+ echo $x;
+endif;
+
+switch ( $x ):
+ case 2:
+ echo $x;
+ break;
+endswitch;
+
+while ( $x > 2 ):
+ echo $i;
+endwhile;
diff --git
a/MediaWiki/Tests/files/ControlStructures/if_else_structure_fail.php.fixed
b/MediaWiki/Tests/files/ControlStructures/if_else_structure_fail.php.fixed
new file mode 100644
index 0000000..6c855ab
--- /dev/null
+++ b/MediaWiki/Tests/files/ControlStructures/if_else_structure_fail.php.fixed
@@ -0,0 +1,11 @@
+<?php
+
+function wfFooFoo( $a ) {
+ if ( $a ) {
+
+ } elseif ( $a ) {
+
+ } else {
+
+ }
+}
diff --git
a/MediaWiki/Tests/files/ExtraCharacters/extra_characters_before_phpopen_tag_fail.php.fixed
b/MediaWiki/Tests/files/ExtraCharacters/extra_characters_before_phpopen_tag_fail.php.fixed
new file mode 100644
index 0000000..abc2e81
--- /dev/null
+++
b/MediaWiki/Tests/files/ExtraCharacters/extra_characters_before_phpopen_tag_fail.php.fixed
@@ -0,0 +1,7 @@
+someText<?php
+// text before first php open
+function wfFoo() {
+
+}
+?>
+T<?php // this php open tag will in any case be ignored
diff --git
a/MediaWiki/Tests/files/NamingConventions/case_global_name_fail.php.fixed
b/MediaWiki/Tests/files/NamingConventions/case_global_name_fail.php.fixed
new file mode 100644
index 0000000..501e089
--- /dev/null
+++ b/MediaWiki/Tests/files/NamingConventions/case_global_name_fail.php.fixed
@@ -0,0 +1,7 @@
+<?php
+
+function wfFooFoo() {
+ // The below should have capital after wg
+ global $wgsomething;
+ $wgsomething = 5;
+}
diff --git
a/MediaWiki/Tests/files/NamingConventions/wg_global_name2_fail.php.fixed
b/MediaWiki/Tests/files/NamingConventions/wg_global_name2_fail.php.fixed
new file mode 100644
index 0000000..c05f938
--- /dev/null
+++ b/MediaWiki/Tests/files/NamingConventions/wg_global_name2_fail.php.fixed
@@ -0,0 +1,8 @@
+<?php
+
+function wfFooFoo() {
+ // The first global is fine, the second isn't
+ global $wgContLang, $LocalInterwikis;
+ $wgContLang = 'en';
+ $LocalInterwikis = false;
+}
diff --git
a/MediaWiki/Tests/files/NamingConventions/wg_global_name_fail.php.fixed
b/MediaWiki/Tests/files/NamingConventions/wg_global_name_fail.php.fixed
new file mode 100644
index 0000000..2f00882
--- /dev/null
+++ b/MediaWiki/Tests/files/NamingConventions/wg_global_name_fail.php.fixed
@@ -0,0 +1,7 @@
+<?php
+
+function wfFooFoo() {
+ // The below should start with wg...
+ global $someotherglobal;
+ $someotherglobal = 5;
+}
diff --git a/MediaWiki/Tests/files/Usage/dir_usage_fail.php.fixed
b/MediaWiki/Tests/files/Usage/dir_usage_fail.php.fixed
new file mode 100644
index 0000000..79aa52b
--- /dev/null
+++ b/MediaWiki/Tests/files/Usage/dir_usage_fail.php.fixed
@@ -0,0 +1,4 @@
+<?php
+
+$tmp = __DIR__;
+echo "Done";
diff --git
a/MediaWiki/Tests/files/VariableAnalysis/unused_global_variables_fail.php.fixed
b/MediaWiki/Tests/files/VariableAnalysis/unused_global_variables_fail.php.fixed
new file mode 100644
index 0000000..4c47bdf
--- /dev/null
+++
b/MediaWiki/Tests/files/VariableAnalysis/unused_global_variables_fail.php.fixed
@@ -0,0 +1,6 @@
+<?php
+
+function wfFooFoo() {
+ // The global variable is not used
+ global $wgSomething;
+}
diff --git
a/MediaWiki/Tests/files/WhiteSpace/multiple_empty_lines_fail.php.fixed
b/MediaWiki/Tests/files/WhiteSpace/multiple_empty_lines_fail.php.fixed
new file mode 100644
index 0000000..528a829
--- /dev/null
+++ b/MediaWiki/Tests/files/WhiteSpace/multiple_empty_lines_fail.php.fixed
@@ -0,0 +1,10 @@
+<?php
+$a = true;
+
+$b = false;
+
+function wfFoo() {
+ $a = 1;
+
+ $b = 2;
+}
diff --git a/MediaWiki/Tests/files/WhiteSpace/short_array_fail.php.fixed
b/MediaWiki/Tests/files/WhiteSpace/short_array_fail.php.fixed
new file mode 100644
index 0000000..2537b73
--- /dev/null
+++ b/MediaWiki/Tests/files/WhiteSpace/short_array_fail.php.fixed
@@ -0,0 +1,6 @@
+<?php
+
+$a = [];
+$b = [ 'faaaail' ];
+$c = [];
+$d = [ 'faaaaail', ];
diff --git
a/MediaWiki/Tests/files/WhiteSpace/space_after_delim_singleline_comment_fail.php.fixed
b/MediaWiki/Tests/files/WhiteSpace/space_after_delim_singleline_comment_fail.php.fixed
new file mode 100644
index 0000000..193ba31
--- /dev/null
+++
b/MediaWiki/Tests/files/WhiteSpace/space_after_delim_singleline_comment_fail.php.fixed
@@ -0,0 +1,8 @@
+<?php
+// a comment with tabs instead of a space
+//
+# yet another comment with tabs instead of a space.
+#
+// A comment without a space
+# Yup, no spaces.
+
diff --git
a/MediaWiki/Tests/files/WhiteSpace/space_before_parens_fail.php.fixed
b/MediaWiki/Tests/files/WhiteSpace/space_before_parens_fail.php.fixed
new file mode 100644
index 0000000..8b8cfbd
--- /dev/null
+++ b/MediaWiki/Tests/files/WhiteSpace/space_before_parens_fail.php.fixed
@@ -0,0 +1,5 @@
+<?php
+function wfBar() {
+ wfFoo();
+ $a = [];
+}
diff --git a/MediaWiki/Tests/files/WhiteSpace/spacey_parenthesis_fail.php.fixed
b/MediaWiki/Tests/files/WhiteSpace/spacey_parenthesis_fail.php.fixed
new file mode 100644
index 0000000..18aa440
--- /dev/null
+++ b/MediaWiki/Tests/files/WhiteSpace/spacey_parenthesis_fail.php.fixed
@@ -0,0 +1,8 @@
+<?php
+
+function wfFooBar( $a, $b ) {
+ $a->foo();
+ $a->foo( $b );
+ $a->foo( $b );
+ $c = [];
+}
diff --git a/composer.json b/composer.json
index 4377cc0..99d5ac1 100644
--- a/composer.json
+++ b/composer.json
@@ -10,7 +10,8 @@
},
"require-dev": {
"jakub-onderka/php-parallel-lint": "0.9.*",
- "phpunit/phpunit": "~4.1"
+ "phpunit/phpunit": "~4.1",
+ "mikey179/vfsStream": "~1.6"
},
"scripts": {
"test": [
--
To view, visit https://gerrit.wikimedia.org/r/291001
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie7102faf842f56e6b93b04f02e90606f19fcaee1
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/tools/codesniffer
Gerrit-Branch: master
Gerrit-Owner: EBernhardson <[email protected]>
Gerrit-Reviewer: Addshore <[email protected]>
Gerrit-Reviewer: Legoktm <[email protected]>
Gerrit-Reviewer: Lethexie <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
