#764 Fix crash on parse some control chars #771

adamfilipow92 · 2020-05-07T15:16:27Z

Fixes #764

Fix crash on parse some control chars

detekt issues fix

bootstraponline · 2020-05-07T16:11:28Z

test_runner/src/test/kotlin/ftl/reports/xml/JUnitXmlTest.kt

+ """.trimIndent()
+
+ parseAllSuitesXml(crashingAllSuitesMessage)
+ parseOneSuiteXml(crashingOneSuiteMessage)


should we also assert that the parsed value matches the expected value? Currently this test will pass as long as parsing doesn't fail. We probably want to assert <failure>... is decoded correctly as well.

You have right, Im add asserts and split tests to two. One for one suite and one for all suites

Add asserts in test and replace Files.readString to Files.readAllBytes for compability with java 1.8

bootstraponline · 2020-05-07T17:32:37Z

test_runner/src/main/kotlin/ftl/reports/xml/JUnitXml.kt

 if (!path.toFile().exists()) throw RuntimeException("$path doesn't exist!")
- return Files.readAllBytes(path)
+ return String(Files.readAllBytes(path))


I'd like to merge #767 before merging this pull request.

I'd like to merge #767 before merging this pull request.

Done

bootstraponline · 2020-05-07T17:35:40Z

test_runner/src/main/kotlin/ftl/reports/xml/preprocesor/XmlPreprocessor.kt

+import org.apache.commons.text.StringEscapeUtils
+
+fun fixHtmlCodes(data: String): String {
+ val isoHtmlCodesToReplace = listOf(0x00..0x1F).union(listOf(0x7F..0x9F)).flatten()


Is there some document we can reference in a comment about where these magic numbers come from?

The other thought I had is, are we sure there's no helper already defined in a library that does what we need? Maybe this is a special case so the custom code is justified.

I this case we need to get list of control chars and convert them to html codes with avoid white chars. I cant find method doing this.
About magic numbers: Im added enum with I hope intuitive names. Enum on the top has comment about destination and link to utf-8 table to prove my idea.

bootstraponline · 2020-05-07T17:36:30Z

test_runner/src/test/kotlin/ftl/reports/xml/JUnitXmlTest.kt

+ </testsuites>
+ """.trimIndent()
+ val oneSuiteXml = parseOneSuiteXml(crashingOneSuiteMessage).xmlToString().trimIndent()
+ Assert.assertEquals("One Suite Messages should be the same!", expectedOneSuiteMessage, oneSuiteXml)


The tests look great! Thanks for updating these.

Fix crash on parse some control chars

detekt issues fix

Add asserts in test and replace Files.readString to Files.readAllBytes for compability with java 1.8

jan-goral · 2020-05-07T20:29:18Z

test_runner/src/main/kotlin/ftl/reports/xml/preprocesor/XmlPreprocessor.kt

+
+import org.apache.commons.text.StringEscapeUtils
+
+fun fixHtmlCodes(data: String): String {


IMO expressions > blocks ;)

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, "") }

unions are not mandatory for those values

I am also for @jan-gogo version 👍

Thanks! Changed to better version :)

pawelpasterz · 2020-05-08T07:48:02Z

test_runner/buildSrc/src/main/kotlin/Deps.kt

@@ -74,6 +74,8 @@ object Versions {

 // https://github.com/mockk/mockk
 const val MOCKK = "1.9.3"
+
+ const val COMMON_TEXT = "1.7"


Please, add here link to repo or equivalent, thanks.

Comment added

pawelpasterz

Please remove should_exists.txt file from PR.

For the future PRs, let's rebase branch once new commits have appeared in master. I think it's good to have PR up-to-date with master in general. It could be just me but in you PR i see also code from @jan-gogo PR a that was a bit confusing.
Anyway, thanks!

* Fail fast when results-dir is incorrect

Fix crash on parse some control chars

detekt issues fix

Add asserts in test and replace Files.readString to Files.readAllBytes for compability with java 1.8

remove should_exists

adamfilipow92 · 2020-05-08T11:38:25Z

Please remove should_exists.txt file from PR.

Removed :)

pawelpasterz · 2020-05-08T12:37:48Z

@adamfilipow92
Nice work! Remember to squash commits before you merge, thanks!

* #764 Fix crash on parse some control chars * #764 detekt issues fix detekt issues fix * Asserts in tests Add asserts in test and replace Files.readString to Files.readAllBytes for compability with java 1.8 * Add documentation of magicial numers * set name of UtfControlChars enum to UtfControlCharRanges * #764 Fix crash on parse some control chars * #764 detekt issues fix detekt issues fix * Asserts in tests Add asserts in test and replace Files.readString to Files.readAllBytes for compability with java 1.8 * Add documentation of magicial numers * set name of UtfControlChars enum to UtfControlCharRanges * Detekt suggestions * Create should_exists.txt * #764 Fix crash on parse some control chars * #764 detekt issues fix detekt issues fix * Asserts in tests Add asserts in test and replace Files.readString to Files.readAllBytes for compability with java 1.8 * Add documentation of magicial numers * set name of UtfControlChars enum to UtfControlCharRanges * Fail fast when results-dir is incorrect (#772) * Fail fast when results-dir is incorrect * #764 Change XmlPreprocessor more functional and remove should_exists * Add info about issue to release notes Co-authored-by: Adam <[email protected]> Co-authored-by: Jan Góral <[email protected]>

#764

f182c8c

Fix crash on parse some control chars

jan-goral changed the title ~~#764~~ #764 Fix crash on parse some control chars May 7, 2020

jan-goral assigned jan-goral and adamfilipow92 and unassigned jan-goral May 7, 2020

#764 detekt issues fix

6be0d1f

detekt issues fix

bootstraponline reviewed May 7, 2020

View reviewed changes

bootstraponline requested review from pawelpasterz and jan-goral May 7, 2020 16:11

Asserts in tests

10a1799

Add asserts in test and replace Files.readString to Files.readAllBytes for compability with java 1.8

bootstraponline reviewed May 7, 2020

View reviewed changes

adamfilipow92 added 10 commits May 7, 2020 20:03

Add documentation of magicial numers

9387a48

set name of UtfControlChars enum to UtfControlCharRanges

ee575ae

#764

b36c3b5

Fix crash on parse some control chars

#764 detekt issues fix

9b025ac

detekt issues fix

Asserts in tests

77b574b

Add asserts in test and replace Files.readString to Files.readAllBytes for compability with java 1.8

Add documentation of magicial numers

1ba4a77

set name of UtfControlChars enum to UtfControlCharRanges

795f9e2

Detekt suggestions

58074c3

Create should_exists.txt

4a484aa

Merge branch 'bug/#764' of https://github.com/Flank/flank into bug/#764

9a6f328

bootstraponline previously approved these changes May 7, 2020

View reviewed changes

jan-goral previously approved these changes May 7, 2020

View reviewed changes

pawelpasterz reviewed May 8, 2020

View reviewed changes

pawelpasterz suggested changes May 8, 2020

View reviewed changes

jan-goral and others added 3 commits May 8, 2020 13:30

Fail fast when results-dir is incorrect (#772)

2e9f925

* Fail fast when results-dir is incorrect

#764

da1fa2a

Fix crash on parse some control chars

#764 detekt issues fix

dfd80fb

detekt issues fix

adamfilipow92 added 4 commits May 8, 2020 13:30

Asserts in tests

793b97e

Add asserts in test and replace Files.readString to Files.readAllBytes for compability with java 1.8

Add documentation of magicial numers

6f8fc9e

set name of UtfControlChars enum to UtfControlCharRanges

5350fc3

#764 Change XmlPreprocessor more functional and

df19d79

remove should_exists

adamfilipow92 dismissed stale reviews from jan-goral and bootstraponline via df19d79 May 8, 2020 11:30

adamfilipow92 and others added 2 commits May 8, 2020 13:53

Add info about issue to release notes

6c60453

Merge branch 'master' into bug/#764

e757e02

jan-goral approved these changes May 8, 2020

View reviewed changes

pawelpasterz approved these changes May 8, 2020

View reviewed changes

adamfilipow92 merged commit 3d960c7 into master May 8, 2020

adamfilipow92 deleted the bug/#764 branch May 8, 2020 13:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#764 Fix crash on parse some control chars #771

#764 Fix crash on parse some control chars #771

adamfilipow92 commented May 7, 2020 •

edited by jan-goral

Loading

bootstraponline May 7, 2020

adamfilipow92 May 7, 2020

bootstraponline May 7, 2020

jan-goral May 7, 2020

bootstraponline May 7, 2020

bootstraponline May 7, 2020

adamfilipow92 May 7, 2020

bootstraponline May 7, 2020

jan-goral May 7, 2020

pawelpasterz May 8, 2020

adamfilipow92 May 8, 2020

pawelpasterz May 8, 2020

adamfilipow92 May 8, 2020

pawelpasterz left a comment

adamfilipow92 commented May 8, 2020

pawelpasterz commented May 8, 2020


		import org.apache.commons.text.StringEscapeUtils

		fun fixHtmlCodes(data: String): String {

#764 Fix crash on parse some control chars #771

#764 Fix crash on parse some control chars #771

Conversation

adamfilipow92 commented May 7, 2020 • edited by jan-goral Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pawelpasterz left a comment

Choose a reason for hiding this comment

adamfilipow92 commented May 8, 2020

pawelpasterz commented May 8, 2020

adamfilipow92 commented May 7, 2020 •

edited by jan-goral

Loading