EBernhardson has uploaded a new change for review.
https://gerrit.wikimedia.org/r/291001
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. Again 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.
.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, 204 insertions(+), 2 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/tools/codesniffer
refs/changes/01/291001/1
diff --git a/MediaWiki/Tests/MediaWikiStandardTest.php
b/MediaWiki/Tests/MediaWikiStandardTest.php
index 44f57a3..9885698 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,28 @@
$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 {
+ unset( $tests[$idx] );
+ }
+ }
+ 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..a303003
--- /dev/null
+++ b/MediaWiki/Tests/MediaWikiTestHelper.php
@@ -0,0 +1,62 @@
+<?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 );
+ }
+
+ // 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], $this->vfsRoot->getChild(
'phpcbf-fixed.diff' )->getContent() );
+ 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: newchange
Gerrit-Change-Id: Ie7102faf842f56e6b93b04f02e90606f19fcaee1
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/tools/codesniffer
Gerrit-Branch: master
Gerrit-Owner: EBernhardson <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits