Summary: | Wikitext stuck in a loop | ||||||
---|---|---|---|---|---|---|---|
Product: | z_Archived | Reporter: | Calum L <caluml> | ||||
Component: | Mylyn | Assignee: | Jeremie Bresson <dev> | ||||
Status: | RESOLVED FIXED | QA Contact: | David Green <greensopinion> | ||||
Severity: | normal | ||||||
Priority: | P1 | CC: | a.c.kalker, dev, steffen.pingel | ||||
Version: | unspecified | ||||||
Target Milestone: | 2.0 | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
URL: | http://fi.wikipedia.org/w/api.php?action=query&prop=revisions&rvprop=content&rvsection=0&format=xml&titles=Dio | ||||||
Whiteboard: | |||||||
Attachments: |
|
Description
Calum L
2013-09-18 04:02:40 EDT
Created attachment 235587 [details]
JUnit test which currently fails
Thanks for the bug Calum. Can you add the specific JVM version that you're using to this bug? Sometimes that makes a difference since we're using regular expressions. Hey David, JDK 1.6.0_25 seems to be what Eclipse is configured to run currently. java version "1.6.0_25" Java(TM) SE Runtime Environment (build 1.6.0_25-b06) Java HotSpot(TM) 64-Bit Server VM (build 20.0-b11, mixed mode) It looks like the Oracle JDK, which is strange, as I'd usually use the OpenJDK one. I have this file in my Downloads, which must be what I used. jdk-6u25-linux-x64.bin I think I had a problem in the OpenJDK one a while ago, and perhaps I switched to the Oracle one to resolve it. I followed the code looping around, and it didn't seem to be to do with regex - but I didn't look too deeply. Calum Reduced example: public void testParserShouldNotTimeout() { String input = "<!-- X -->Lorem<!-- Y -->Ipsum\n" + "<!-- Z -->"; String html = parser.parseToHtml(input); TestUtil.println(html); assertTrue(html.contains("XXX")); //TODO fix the assert. } Added in class MediaWikiLanguageTest. The assertTrue(..) line is never executed. I will try to debug it this evening, to see what is going wrong. Here a Gerrit Change with the proposed JUnit test and a solution: https://git.eclipse.org/r/17659 It does the job but I am not sure this is the solution. Two questions: 1/ Are you sure that CommentBlock should be the ParagraphBreakingBlocks? Why is there a NESTED_BLOCK_START_PATTERN after the ParagraphBreakingBlocks Check? I wanted to have a look with a Code Coverage tool, because I do not see when this code is used. 2/ Are you sure that findCloseOffset can return -1? It seems to me, that by returning -1, the ParagraphBlock instance is never poped from the nestedBlocks stack. => Maybe with an other value, my change in canResume(..) would not be necessary. Jeremie, thanks for your excellent work. I've pushed an updated patchset to the review that includes your test but provides an alternate solution. Please take a look and let me know what you think. Keeping closed blocks in nestedBlocks is wrong. Your solution that pop them from the stack is good. I still think that the nested-protocol in MediaWiki ParagraphBlock is not so well implemented: findCloseOffset(..) should not always return -1. Your change makes MarkupLanguage more robust -> this is good. I've merged review 17659, thanks Jeremie. Calumn I'll close this one as resolved/fixed since the change fixes the problem in our tests. If you can confirm that the fix works for you that would be great. Feel free to reopen this bug if the problem comes up again. *** Bug 402248 has been marked as a duplicate of this bug. *** |