Differential D25847
Fix regression in Remarkup unit tests and harden Authored by valerio.bozzolan on Thu, Dec 5, 15:52.
Details Fix regressions in two unit tests, because of a CSS addition from here: https://we.phorge.it/rP349f006904fabf1d4df31ff4840502af3ab379a7 Some unit tests were missing the new CSS class 'remarkup-link-ext'. Added. Also, we do not consider anymore 'mailto:info@example.com' as an internal link. We hardened a bit the unit tests to also include mobile numbers in links, and alien protocols. Run these unit tests, and they finally work again: arc unit ./src/infrastructure/parser/__tests__/PhutilPygmentizeParserTestCase.php arc unit ./src/infrastructure/markup/remarkup/__tests__/PhutilRemarkupEngineTestCase.php Additionally, run this, that has new tests added: arc unit ./src/infrastructure/parser/__tests__/PhutilURIHelperTestCase.php Also run this, check that the class tree is already correct: arc liberate Have fun with the green lights.
Diff Detail
Event TimelineComment Actions I applied this patch locally on top of git master and output does not complain anymore about 'link-brackets.txt' (thus it's correct) but fails in link-edge-cases.txt now (thus it's likely not complete): FAIL PhutilRemarkupEngineTestCase::testEngine Assertion failed, expected values to be equal (at PhutilRemarkupEngineTestCase.php:66): Failed to markup HTML in file 'link-edge-cases.txt'. Expected vs Actual Output Diff --- Old Value +++ New Value @@ -1,11 +1,11 @@ -'<p><a href="http://www.example.com/" class="remarkup-link" target="_blank" rel="noreferrer">http://www.example.com/</a></p> +'<p><a href="http://www.example.com/" class="remarkup-link remarkup-link-ext" target="_blank" rel="noreferrer">http://www.example.com/</a></p> -<p>(<a href="http://www.example.com/" class="remarkup-link" target="_blank" rel="noreferrer">http://www.example.com/</a>)</p> +<p>(<a href="http://www.example.com/" class="remarkup-link remarkup-link-ext" target="_blank" rel="noreferrer">http://www.example.com/</a>)</p> -<p><a href="http://www.example.com/" class="remarkup-link" target="_blank" rel="noreferrer">http://www.example.com/</a></p> +<p><a href="http://www.example.com/" class="remarkup-link remarkup-link-ext" target="_blank" rel="noreferrer">http://www.example.com/</a></p> -<p><a href="http://www.example.com/wiki/example_(disambiguation)" class="remarkup-link" target="_blank" rel="noreferrer">http://www.example.com/wiki/example_(disambiguation)</a></p> +<p><a href="http://www.example.com/wiki/example_(disambiguation)" class="remarkup-link remarkup-link-ext" target="_blank" rel="noreferrer">http://www.example.com/wiki/example_(disambiguation)</a></p> -<p>(example <a href="http://www.example.com/" class="remarkup-link" target="_blank" rel="noreferrer">http://www.example.com/</a>)</p> +<p>(example <a href="http://www.example.com/" class="remarkup-link remarkup-link-ext" target="_blank" rel="noreferrer">http://www.example.com/</a>)</p> -<p>Quick! <a href="http://www.example.com/" class="remarkup-link" target="_blank" rel="noreferrer">http://www.example.com/</a>!</p>' +<p>Quick! <a href="http://www.example.com/" class="remarkup-link remarkup-link-ext" target="_blank" rel="noreferrer">http://www.example.com/</a>!</p>' Comment Actions
You are right. Marking as revision not ready. But this is becoming a rabbit hole. Seems the Env/Configs are not configured correctly in unit tests, and we really need them: arc unit src/infrastructure/markup/remarkup/__tests__/PhutilRemarkupEngineTestCase.php
FAIL PhutilRemarkupEngineTestCase::testEngine
EXCEPTION (Exception): Trying to read configuration "phabricator.production-uri" before configuration has been initialized.
#0 /home/boz/repos/phorge/src/infrastructure/env/PhabricatorEnv.php(423): PhabricatorEnv::getEnvConfig('...')
#1 /home/boz/repos/phorge/src/infrastructure/env/PhabricatorEnv.php(447): PhabricatorEnv::getProductionURI('...')
#2 /home/boz/repos/phorge/src/infrastructure/env/PhabricatorEnv.php(441): PhabricatorEnv::getSelfURIMap()
#3 /home/boz/repos/phorge/src/infrastructure/parser/PhutilURIHelper.php(43): PhabricatorEnv::isSelfURI('...')
#4 /home/boz/repos/phorge/src/infrastructure/markup/markuprule/PhutilRemarkupDocumentLinkRule.php(80): PhutilURIHelper->isSelf()
#5 /home/boz/repos/phorge/src/infrastructure/markup/markuprule/PhutilRemarkupDocumentLinkRule.php(186): PhutilRemarkupDocumentLinkRule->renderHyperlink('...', '...')
#6 /home/boz/repos/phorge/src/infrastructure/markup/markuprule/PhutilRemarkupDocumentLinkRule.php(133): PhutilRemarkupDocumentLinkRule->markupDocumentLink(Array)
#7 [internal function]: PhutilRemarkupDocumentLinkRule->markupAlternateLink(Array)
#8 /home/boz/repos/phorge/src/infrastructure/markup/markuprule/PhutilRemarkupDocumentLinkRule.php(17): preg_replace_callback('...', Array, '...')
#9 /home/boz/repos/phorge/src/infrastructure/markup/blockrule/PhutilRemarkupBlockRule.php(92): PhutilRemarkupDocumentLinkRule->apply('...')
#10 /home/boz/repos/phorge/src/infrastructure/markup/blockrule/PhutilRemarkupDefaultBlockRule.php(17): PhutilRemarkupBlockRule->applyRules('...')
#11 /home/boz/repos/phorge/src/infrastructure/markup/remarkup/PhutilRemarkupEngine.php(283): PhutilRemarkupDefaultBlockRule->markupText('...', NULL)
#12 /home/boz/repos/phorge/src/infrastructure/markup/remarkup/PhutilRemarkupEngine.php(140): PhutilRemarkupEngine->markupBlock(Array)
#13 /home/boz/repos/phorge/src/infrastructure/markup/remarkup/PhutilRemarkupEngine.php(106): PhutilRemarkupEngine->preprocessText('...')
#14 /home/boz/repos/phorge/src/infrastructure/markup/remarkup/__tests__/PhutilRemarkupEngineTestCase.php(56): PhutilRemarkupEngine->markupText('...')
#15 /home/boz/repos/phorge/src/infrastructure/markup/remarkup/__tests__/PhutilRemarkupEngineTestCase.php(13): PhutilRemarkupEngineTestCase->markupText('...')
#16 /home/boz/repos/arcanist/src/unit/engine/phutil/PhutilTestCase.php(639): PhutilRemarkupEngineTestCase->testEngine()
#17 /home/boz/repos/arcanist/src/unit/engine/PhutilUnitTestEngine.php(69): PhutilTestCase->run()
#18 /home/boz/repos/arcanist/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php(148): PhutilUnitTestEngine->run()
#19 /home/boz/repos/arcanist/src/workflow/ArcanistUnitWorkflow.php(170): ArcanistConfigurationDrivenUnitTestEngine->run()
#20 /home/boz/repos/arcanist/scripts/arcanist.php(427): ArcanistUnitWorkflow->run()
#21 {main}I mark as changes planned, but I really don't know what to do. Let's talk in new task T15973: Fix unit test PhutilRemarkupEngineTestCase
| ||||||||||||||||||||