-
Notifications
You must be signed in to change notification settings - Fork 119
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#764 Fix crash on parse some control chars #771
Merged
Merged
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
f182c8c
#764
adamfilipow92 6be0d1f
#764 detekt issues fix
adamfilipow92 10a1799
Asserts in tests
adamfilipow92 9387a48
Add documentation of magicial numers
adamfilipow92 ee575ae
set name of UtfControlChars enum to UtfControlCharRanges
adamfilipow92 b36c3b5
#764
adamfilipow92 9b025ac
#764 detekt issues fix
adamfilipow92 77b574b
Asserts in tests
adamfilipow92 1ba4a77
Add documentation of magicial numers
adamfilipow92 795f9e2
set name of UtfControlChars enum to UtfControlCharRanges
adamfilipow92 58074c3
Detekt suggestions
adamfilipow92 4a484aa
Create should_exists.txt
adamfilipow92 9a6f328
Merge branch 'bug/#764' of https://github.com/Flank/flank into bug/#764
adamfilipow92 2e9f925
Fail fast when results-dir is incorrect (#772)
jan-goral da1fa2a
#764
adamfilipow92 dfd80fb
#764 detekt issues fix
adamfilipow92 793b97e
Asserts in tests
adamfilipow92 6f8fc9e
Add documentation of magicial numers
adamfilipow92 5350fc3
set name of UtfControlChars enum to UtfControlCharRanges
adamfilipow92 df19d79
#764 Change XmlPreprocessor more functional and
adamfilipow92 6c60453
Add info about issue to release notes
adamfilipow92 e757e02
Merge branch 'master' into bug/#764
adamfilipow92 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
13 changes: 13 additions & 0 deletions
13
test_runner/src/main/kotlin/ftl/reports/xml/preprocesor/UtfControlCharRanges.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package ftl.reports.xml.preprocesor | ||
|
||
/** | ||
* Numbers come from ascii table https://www.utf8-chartable.de. | ||
* and represents control chars. We need to avoid characters in ranges CONTROL_TOP_START..CONTROL_TOP_END and | ||
* CONTROL_BOTTOM_START..CONTROL_BOTTOM_END because chars from that range escaped to html causing parsing errors. | ||
* */ | ||
enum class UtfControlCharRanges(val charValue: Int) { | ||
CONTROL_TOP_START(0x00), | ||
CONTROL_TOP_END(0x1F), | ||
CONTROL_BOTTOM_START(0x7F), | ||
CONTROL_BOTTOM_END(0x9F) | ||
} |
11 changes: 11 additions & 0 deletions
11
test_runner/src/main/kotlin/ftl/reports/xml/preprocesor/XmlPreprocessor.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
package ftl.reports.xml.preprocesor | ||
|
||
import org.apache.commons.text.StringEscapeUtils | ||
|
||
fun fixHtmlCodes(data: String): String = listOf( | ||
UtfControlCharRanges.CONTROL_TOP_START.charValue..UtfControlCharRanges.CONTROL_TOP_END.charValue, | ||
UtfControlCharRanges.CONTROL_BOTTOM_START.charValue..UtfControlCharRanges.CONTROL_BOTTOM_END.charValue | ||
).flatten() | ||
.map { StringEscapeUtils.escapeXml11(it.toChar().toString()) } | ||
.filter { it.startsWith("&#") } | ||
.fold(data) { fixedStr: String, isoControlCode: String -> fixedStr.replace(isoControlCode, "") } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ package ftl.reports.xml | |
|
||
import com.google.common.truth.Truth.assertThat | ||
import ftl.test.util.TestHelper.normalizeLineEnding | ||
import org.junit.Assert | ||
import java.nio.file.Paths | ||
import org.junit.Test | ||
|
||
|
@@ -73,7 +74,8 @@ junit.framework.Assert.fail(Assert.java:50)</failure> | |
|
||
@Test | ||
fun `merge ios`() { | ||
val merged = parseAllSuitesXml(iosPassXml).merge(parseAllSuitesXml(iosFailXml)).xmlToString().normalizeLineEnding() | ||
val merged = | ||
parseAllSuitesXml(iosPassXml).merge(parseAllSuitesXml(iosFailXml)).xmlToString().normalizeLineEnding() | ||
val expected = """ | ||
<?xml version='1.0' encoding='UTF-8' ?> | ||
<testsuites> | ||
|
@@ -94,7 +96,8 @@ junit.framework.Assert.fail(Assert.java:50)</failure> | |
|
||
@Test | ||
fun `Merge iOS large time`() { | ||
val merged = parseAllSuitesXml(iosLargeNum).merge(parseAllSuitesXml(iosLargeNum)).xmlToString().normalizeLineEnding() | ||
val merged = | ||
parseAllSuitesXml(iosLargeNum).merge(parseAllSuitesXml(iosLargeNum)).xmlToString().normalizeLineEnding() | ||
|
||
val expected = """ | ||
<?xml version='1.0' encoding='UTF-8' ?> | ||
|
@@ -423,7 +426,8 @@ junit.framework.Assert.fail(Assert.java:50)</failure> | |
// * c() failed in newRun and passed in oldRun. timing info copied over from oldRun | ||
// * d() was skipped in newRun and successful in oldRun. d() is excluded from the merged result | ||
|
||
val merged = parseAllSuitesXml(newRun).mergeTestTimes(parseAllSuitesXml(oldRun)).xmlToString().normalizeLineEnding() | ||
val merged = | ||
parseAllSuitesXml(newRun).mergeTestTimes(parseAllSuitesXml(oldRun)).xmlToString().normalizeLineEnding() | ||
val expected = """ | ||
<?xml version='1.0' encoding='UTF-8' ?> | ||
<testsuites> | ||
|
@@ -437,4 +441,64 @@ junit.framework.Assert.fail(Assert.java:50)</failure> | |
""".trimIndent() | ||
assertThat(merged).isEqualTo(expected) | ||
} | ||
|
||
@Test | ||
fun `parse ftl quirks in all suites`() { | ||
val crashingAllSuitesMessage = """ | ||
<?xml version='1.0' encoding='UTF-8' ?> | ||
<testsuites> | ||
<testsuite name="EarlGreyExampleSwiftTests" tests="3" failures="0" errors="0" skipped="0" time="10.0" hostname="localhost"> | ||
<testcase name="a()" classname="a" time="1.0"> | ||
<failure> java.net.ConnectException: Failed to connect to ... at (Coroutine boundary.()</failure> | ||
</testcase> | ||
<testcase name="b()" classname="b" time="2.0"/> | ||
<testcase name="c()" classname="c" time="7.0"/> | ||
</testsuite> | ||
</testsuites> | ||
""".trimIndent() | ||
|
||
val expectedAllSuitesMessage = """ | ||
<?xml version='1.0' encoding='UTF-8' ?> | ||
<testsuites> | ||
<testsuite name="EarlGreyExampleSwiftTests" tests="3" failures="0" errors="0" skipped="0" time="10.0" hostname="localhost"> | ||
<testcase name="a()" classname="a" time="1.0"> | ||
<failure> java.net.ConnectException: Failed to connect to ... at (Coroutine boundary.()</failure> | ||
</testcase> | ||
<testcase name="b()" classname="b" time="2.0"/> | ||
<testcase name="c()" classname="c" time="7.0"/> | ||
</testsuite> | ||
</testsuites> | ||
""".trimIndent() | ||
val allSuitesXml = parseAllSuitesXml(crashingAllSuitesMessage).xmlToString().trimIndent() | ||
Assert.assertEquals("All Suite Messages should be the same!", expectedAllSuitesMessage, allSuitesXml) | ||
} | ||
|
||
@Test | ||
fun `parse ftl quirks in on suite`() { | ||
val crashingOneSuiteMessage = """ | ||
<?xml version='1.0' encoding='UTF-8' ?> | ||
<testsuite name="EarlGreyExampleSwiftTests" tests="3" failures="0" errors="0" skipped="0" time="10.0" hostname="localhost"> | ||
<testcase name="a()" classname="a" time="1.0"> | ||
<failure> java.net.ConnectException: Failed to connect to ... at (Coroutine boundary.()</failure> | ||
</testcase> | ||
<testcase name="b()" classname="b" time="2.0"/> | ||
<testcase name="c()" classname="c" time="7.0"/> | ||
</testsuite> | ||
""".trimIndent() | ||
|
||
val expectedOneSuiteMessage = """ | ||
<?xml version='1.0' encoding='UTF-8' ?> | ||
<testsuites> | ||
<testsuite name="EarlGreyExampleSwiftTests" tests="3" failures="0" errors="0" skipped="0" time="10.0" hostname="localhost"> | ||
<testcase name="a()" classname="a" time="1.0"> | ||
<failure> java.net.ConnectException: Failed to connect to ... at (Coroutine boundary.()</failure> | ||
</testcase> | ||
<testcase name="b()" classname="b" time="2.0"/> | ||
<testcase name="c()" classname="c" time="7.0"/> | ||
</testsuite> | ||
</testsuites> | ||
""".trimIndent() | ||
val oneSuiteXml = parseOneSuiteXml(crashingOneSuiteMessage).xmlToString().trimIndent() | ||
Assert.assertEquals("One Suite Messages should be the same!", expectedOneSuiteMessage, oneSuiteXml) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The tests look great! Thanks for updating these. |
||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to merge #767 before merging this pull request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done