From 93073d4632222e2524117a987b45e7e90db48d17 Mon Sep 17 00:00:00 2001 From: "C. Scott Ananian" Date: Wed, 22 Feb 2023 15:49:45 -0500 Subject: [PATCH] ParserTestRunner: handle metadata output as separate section If a ParserTest mixes HTML output and metadata properties, it can complicate HTML normalization and other test processes, especially for Parsoid-mode bidirectional tests. Support splitting metadata output into a separate section, named `!! metadata`, with the standard options for legacy and parsoid variants, like `!! metadata/php` and `!! metadata/parsoid` and `!! metadata/parsoid+integrated` etc. For compatibility, if the metadata flags are present on the test and the new section is not present, we'll continue to handle the metadata output as we have before, aka append or prepend the metadata to the HTML. Code search for uses of these options (uses in parsoid and core can be ignored; uses of 'pst' are harmless when they are not combined with another option): https://codesearch.wmcloud.org/search/?q=%28%5E%7C%20%29%28%28showtitle%7Cshowindicators%7Cill%7Ccat%7Cpst%7Cshowflags%29%28%20%7C%24%29%7C%28extension%3D%7Cproperty%3D%29%29&i=nope&files=%5Etests%2Fparser%2F.*%5C.txt&excludeFiles=&repos= Change-Id: I845694d4f2109a8b9125410e8533ca69bbea50fa --- tests/parser/ParserTestRunner.php | 127 ++++++++++++++++--------- tests/parser/interlanguageLinks.txt | 3 +- tests/parser/magicWords.txt | 60 ++++++++---- tests/parser/parserTests.txt | 138 ++++++++++++++++++++-------- tests/parser/pst.txt | 3 +- tests/parser/toc.txt | 36 ++++---- 6 files changed, 244 insertions(+), 123 deletions(-) diff --git a/tests/parser/ParserTestRunner.php b/tests/parser/ParserTestRunner.php index 9b8bd266b82..2bb1adec180 100644 --- a/tests/parser/ParserTestRunner.php +++ b/tests/parser/ParserTestRunner.php @@ -986,13 +986,17 @@ class ParserTestRunner { return "Test doesn't match filter"; } // Skip parsoid-only tests if running in a legacy test mode - if ( $test->legacyHtml === null ) { + if ( + $test->legacyHtml === null && + self::getLegacyMetadataSection( $test ) === null + ) { // A Parsoid-only test should have one of the following sections if ( isset( $test->sections['html/parsoid'] ) || isset( $test->sections['html/parsoid+integrated'] ) || isset( $test->sections['html/parsoid+standalone'] ) || - isset( $test->sections['wikitext/edited'] ) + isset( $test->sections['wikitext/edited'] ) || + self::getParsoidMetadataSection( $test ) !== null ) { if ( $mode->isLegacy() ) { // Not an error, just skip this test if we're in @@ -1000,14 +1004,39 @@ class ParserTestRunner { return "Parsoid-only test"; } } else { - // This test lacks both a legacy html section and also - // any parsoid-specific html or wikitext/edited section. - $test->error( "Test lacks html section", $test->testName ); + // This test lacks both a legacy html or metadata + // section and also any parsoid-specific html or + // metadata section or wikitext/edited section. + $test->error( "Test lacks html or metadata section", $test->testName ); } } return null; } + public static function getLegacyMetadataSection( ParserTest $test ) { + return // specific results for legacy parser + $test->sections['metadata/php'] ?? + // specific results for legacy parser and parsoid integrated mode + $test->sections['metadata/integrated'] ?? + // generic for all parsers (even standalone) + $test->sections['metadata'] ?? + // missing (== use legacy combined output format) + null; + } + + public static function getParsoidMetadataSection( ParserTest $test ) { + return // specific results for parsoid integrated mode + $test->sections['metadata/parsoid+integrated'] ?? + // specific results for parsoid + $test->sections['metadata/parsoid'] ?? + // specific results for legacy parser and parsoid integrated mode + $test->sections['metadata/integrated'] ?? + // generic for all parsers (even standalone) + $test->sections['metadata'] ?? + // missing (== use legacy combined output format) + null; + } + /** * Compute valid test modes based on requested modes and file-enabled modes * @param array $testModes @@ -1359,14 +1388,19 @@ class ParserTestRunner { $out = preg_replace( '/\s+$/', '', $out ); } } + + $metadataExpected = self::getLegacyMetadataSection( $test ); + $metadataActual = null; if ( $output ) { - $this->addParserOutputInfo( $out, $output, $opts, $title ); + $this->addParserOutputInfo( + $out, $output, $opts, $title, + $metadataExpected, $metadataActual + ); } ScopedCallback::consume( $teardownGuard ); - $expected = $test->legacyHtml; - '@phan-var string $expected'; // assert that this is not null + $expected = $test->legacyHtml ?? ''; if ( count( $this->normalizationFunctions ) ) { $expected = ParserTestResultNormalizer::normalize( $expected, $this->normalizationFunctions ); @@ -1374,56 +1408,59 @@ class ParserTestRunner { } $testResult = new ParserTestResult( $test, $mode, $expected, $out ); + if ( $testResult->isSuccess() && $metadataExpected !== null ) { + $testResult = new ParserTestResult( $test, $mode, $metadataExpected, $metadataActual ?? '' ); + } return $testResult; } /** * Add information from the parser output to the result string * - * @param string &$out - * @param ParserOutput $output - * @param array $opts + * @param string &$out The "actual" parser output + * @param ParserOutput $output The "actual" parser metadata + * @param array $opts Test options * @param Title $title + * @param ?string $metadataExpected The contents of the !!metadata section, + * or null if it is missing + * @param ?string &$metadataActual The "actual" metadata output */ - private function addParserOutputInfo( &$out, ParserOutput $output, array $opts, Title $title ) { + private function addParserOutputInfo( + &$out, ParserOutput $output, array $opts, Title $title, + ?string $metadataExpected, ?string &$metadataActual + ) { + $before = []; + $after = []; + // The "before" entries may contain HTML. if ( isset( $opts['showtitle'] ) ) { if ( $output->getTitleText() ) { $titleText = $output->getTitleText(); } else { $titleText = $title->getPrefixedText(); } - - $out = "$titleText\n$out"; + $before[] = $titleText; } if ( isset( $opts['showindicators'] ) ) { - $indicators = ''; foreach ( $output->getIndicators() as $id => $content ) { - $indicators .= "$id=$content\n"; + $before[] = "$id=$content"; } - $out = $indicators . $out; } if ( isset( $opts['ill'] ) ) { - if ( $out !== '' ) { - $out .= "\n"; - } - $out .= implode( ' ', $output->getLanguageLinks() ); - } elseif ( isset( $opts['cat'] ) ) { + $after[] = implode( ' ', $output->getLanguageLinks() ); + } + + if ( isset( $opts['cat'] ) ) { foreach ( $output->getCategories() as $name => $sortkey ) { - if ( $out !== '' ) { - $out .= "\n"; - } - $out .= "cat=$name sort=$sortkey"; + $after[] = "cat=$name sort=$sortkey"; } } if ( isset( $opts['extension'] ) ) { foreach ( explode( ',', $opts['extension'] ) as $ext ) { - if ( $out !== '' ) { - $out .= "\n"; - } - $out .= "extension[$ext]=" . + $after[] = "extension[$ext]=" . + // XXX should use JsonCodec json_encode( $output->getExtensionData( $ext ), JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE | JSON_PRETTY_PRINT @@ -1433,10 +1470,7 @@ class ParserTestRunner { if ( isset( $opts['property'] ) ) { foreach ( explode( ',', $opts['property'] ) as $prop ) { - if ( $out !== '' ) { - $out .= "\n"; - } - $out .= "property[$prop]=" . + $after[] = "property[$prop]=" . ( $output->getPageProperty( $prop ) ?? '' ); } } @@ -1456,10 +1490,7 @@ class ParserTestRunner { } } sort( $actualFlags ); - if ( $out !== '' ) { - $out .= "\n"; - } - $out .= "flags=" . implode( ', ', $actualFlags ); + $after[] = "flags=" . implode( ', ', $actualFlags ); # In 1.21 we deprecated the use of arbitrary keys for # ParserOutput::setFlag() by extensions; if we find anyone # still doing that complain about it. @@ -1475,14 +1506,24 @@ class ParserTestRunner { // FIXME: We probably want to update this to a different format $sections = $output->getTOCData() !== null ? $output->getTOCData()->getSections() : []; - $toc = []; foreach ( $sections as $s ) { - $toc[] = json_encode( $s->toLegacy() ); + $after[] = json_encode( $s->toLegacy() ); } - if ( $out !== '' ) { - $out .= "\n"; + } + if ( $metadataExpected === null ) { + // legacy format, add $before and $after to $out + if ( $before ) { + $before = implode( "\n", $before ); + $out = "$before\n$out"; } - $out .= implode( "\n", $toc ); + if ( $after ) { + if ( $out && !str_ends_with( $out, "\n" ) ) { + $out .= "\n"; + } + $out .= implode( "\n", $after ); + } + } else { + $metadataActual = implode( "\n", array_merge( $before, $after ) ); } } diff --git a/tests/parser/interlanguageLinks.txt b/tests/parser/interlanguageLinks.txt index 5757ec5aba5..0926b2d7a92 100644 --- a/tests/parser/interlanguageLinks.txt +++ b/tests/parser/interlanguageLinks.txt @@ -235,11 +235,12 @@ ill [[es:]] [[ko:]] +!! metadata +es: !! html/php


ko:

-es: !! html/parsoid diff --git a/tests/parser/magicWords.txt b/tests/parser/magicWords.txt index 34a25a37c8e..4474156ee27 100644 --- a/tests/parser/magicWords.txt +++ b/tests/parser/magicWords.txt @@ -625,10 +625,11 @@ parsoid={ "modes": ["wt2html","wt2wt"] } showflags !! wikitext {{REVISIONID}} +!! metadata/integrated +flags=vary-revision-id !! html/php

1337

-flags=vary-revision-id !! html/parsoid+integrated

1337

!! end @@ -640,10 +641,11 @@ parsoid={ "modes": ["wt2html","wt2wt"] } showflags !! wikitext {{REVISIONID}} +!! metadata/integrated +flags=vary-revision-id !! html/php

1337

-flags=vary-revision-id !! html/parsoid+integrated

1337

!! end @@ -656,10 +658,11 @@ parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true } showflags !! wikitext {{REVISIONTIMESTAMP}} +!! metadata/integrated +flags= !! html/php

19700101000203

-flags= !! html/parsoid+integrated

19700101000203

!! end @@ -672,10 +675,11 @@ parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true } showflags !! wikitext {{REVISIONTIMESTAMP:{{PAGENAME}}}} +!! metadata/integrated +flags= !! html/php

19700101000203

-flags= !! html/parsoid+integrated

19700101000203

!! end @@ -688,10 +692,11 @@ parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true } showflags !! wikitext {{REVISIONTIMESTAMP}} +!! metadata/integrated +flags=vary-revision-timestamp !! html/php

19700101000203

-flags=vary-revision-timestamp !! html/parsoid+integrated

19700101000203

!! end @@ -704,10 +709,11 @@ parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true } showflags !! wikitext {{REVISIONTIMESTAMP:{{PAGENAME}}}} +!! metadata/integrated +flags=vary-revision-timestamp !! html/php

19700101000203

-flags=vary-revision-timestamp !! html/parsoid+integrated

19700101000203

!! end @@ -719,8 +725,9 @@ parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true } showflags !! wikitext {{REVISIONTIMESTAMP:This page does not exist}} -!! html/php +!! metadata/integrated flags= +!! html/php !! html/parsoid+integrated !! end @@ -733,10 +740,11 @@ parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true } showflags !! wikitext {{REVISIONUSER}} +!! metadata/integrated +flags=vary-user !! html/php

127.0.0.1

-flags=vary-user !! html/parsoid+integrated

127.0.0.1

!! end @@ -749,8 +757,9 @@ parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true } showflags !! wikitext {{REVISIONUSER}} -!! html/php +!! metadata/integrated flags=vary-user +!! html/php !! html/parsoid+integrated !! end @@ -763,8 +772,9 @@ parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true } showflags !! wikitext {{REVISIONUSER:{{PAGENAME}}}} -!! html/php +!! metadata/integrated flags=vary-user +!! html/php !! html/parsoid+integrated !! end @@ -776,8 +786,9 @@ parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true } showflags !! wikitext {{REVISIONUSER:This page does not exist}} -!! html/php +!! metadata/integrated flags= +!! html/php !! html/parsoid+integrated !! end @@ -789,8 +800,9 @@ parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true } showflags !! wikitext {{REVISIONUSER}} -!! html/php +!! metadata/integrated flags=vary-user +!! html/php !! html/parsoid+integrated !! end @@ -803,10 +815,11 @@ parsoid={ "modes": ["wt2html","wt2wt"] } showflags !! wikitext {{REVISIONID:{{PAGENAME}}}} +!! metadata/integrated +flags=vary-revision-id !! html/php

1337

-flags=vary-revision-id !! html/parsoid+integrated

1337

!! end @@ -818,8 +831,9 @@ parsoid={ "modes": ["wt2html","wt2wt"] } showflags !! wikitext {{REVISIONID:{{PAGENAME}}}} -!! html/php +!! metadata/integrated flags=vary-revision-id +!! html/php !! html/parsoid+integrated !! end @@ -832,10 +846,11 @@ parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true } showflags !! wikitext {{REVISIONDAY}} +!! metadata/integrated +flags= !! html/php

1

-flags= !! html/parsoid+integrated

1

!! end @@ -848,10 +863,11 @@ parsoid={ "modes": ["wt2html","wt2wt"] } showflags !! wikitext {{REVISIONDAY:{{PAGENAME}}}} +!! metadata/integrated +flags= !! html/php

1

-flags= !! html/parsoid+integrated

1

!! end @@ -864,10 +880,11 @@ parsoid={ "modes": ["wt2html","wt2wt"], "normalizePhp": true } showflags !! wikitext {{REVISIONMONTH}} +!! metadata/integrated +flags= !! html/php

01

-flags= !! html/parsoid+integrated

01

!! end @@ -880,10 +897,11 @@ parsoid={ "modes": ["wt2html","wt2wt"] } showflags !! wikitext {{REVISIONMONTH:{{PAGENAME}}}} +!! metadata/integrated +flags= !! html/php

01

-flags= !! html/parsoid+integrated

01

!! end @@ -896,10 +914,11 @@ parsoid={ "modes": ["wt2html","wt2wt"] } showflags !! wikitext {{REVISIONYEAR:{{PAGENAME}}}} +!! metadata/integrated +flags= !! html/php

1970

-flags= !! html/parsoid+integrated

1970

!! end @@ -912,10 +931,11 @@ parsoid={ "modes": ["wt2html","wt2wt"] } showflags !! wikitext {{PAGESIZE:{{PAGENAME}}}} +!! metadata/integrated +flags=vary-revision-sha1 !! html/php

25

-flags=vary-revision-sha1 !! html/parsoid+integrated

25

!! end diff --git a/tests/parser/parserTests.txt b/tests/parser/parserTests.txt index 7945a9d1d95..b710f1f2321 100644 --- a/tests/parser/parserTests.txt +++ b/tests/parser/parserTests.txt @@ -4055,9 +4055,9 @@ wgAllowDisplayTitle=true wgRestrictDisplayTitle=false !! wikitext {{DISPLAYTITLE:''{{PAGENAME}}''}} -!! html/php +!! metadata Parser test - +!! html/php !! html/parsoid !! end @@ -4074,9 +4074,9 @@ showtitle !! config wgAllowDisplayTitle=true wgRestrictDisplayTitle=false -!! html/php +!! metadata Foo - +!! html/php !! html/parsoid !! end @@ -7876,12 +7876,13 @@ __NOTOC__ ==Section 0== {{sections}} ==Section 4== +!! metadata +flags= !! html/php

Section 0[edit]

Section 1[edit]

Section 2[edit]

Section 4[edit]

-flags= !! end !! test @@ -7889,8 +7890,9 @@ T307691: show-toc flag: no sections !! options showflags !! wikitext -!! html/php +!! metadata flags= +!! html !! end # You can't force a TOC if there aren't any sections @@ -7900,8 +7902,11 @@ T307691: show-toc flag: no sections, but __FORCETOC__ showflags !! wikitext __FORCETOC__ -!! html/php +!! metadata flags= +!! html/php +!! html/parsoid + !! end # Placing a manual __TOC__ doesn't do anything if there aren't any sections @@ -7911,8 +7916,11 @@ T307691: show-toc flag: no sections, but __TOC__ showflags !! wikitext __TOC__ -!! html/php +!! metadata flags= +!! html/php +!! html/parsoid + !! end !! test @@ -7921,8 +7929,11 @@ T307691: show-toc flag: no sections, and __NOTOC__ showflags !! wikitext __NOTOC__ -!! html/php +!! metadata flags= +!! html/php +!! html/parsoid + !! end !! test @@ -7931,9 +7942,12 @@ T307691: show-toc flag: not "enough" sections showflags !! wikitext == One == +!! metadata +flags= !! html/php

One[edit]

-flags= +!! html/parsoid +

One

!! end !! test @@ -7943,6 +7957,8 @@ showflags !! wikitext __FORCETOC__ == One == +!! metadata +flags=show-toc !! html/php

One[edit]

-flags=show-toc +!! html/parsoid + +

One

!! end !! test @@ -7961,6 +7979,8 @@ showflags !! wikitext == One == __TOC__ +!! metadata +flags=show-toc !! html/php

One[edit]

-flags=show-toc +!! html/parsoid +

One

+ !! end !! test @@ -7978,9 +8000,13 @@ showflags !! wikitext __NOTOC__ == One == +!! metadata +flags= !! html/php

One[edit]

-flags= +!! html/parsoid + +

One

!! end !! test @@ -7992,6 +8018,8 @@ showflags === Two === == Three == === Four === +!! metadata +flags=show-toc !! html/php