From 810f630328d8302021773319895d46ecc40bf979 Mon Sep 17 00:00:00 2001 From: ilias Date: Tue, 14 Feb 2023 11:18:31 +0100 Subject: [PATCH 1/6] Add failing test --- tests/ci.js | 6 ++++++ tests/fixtures/tests/InvalidXMLCharacter/Test.elm | 12 ++++++++++++ 2 files changed, 18 insertions(+) create mode 100644 tests/fixtures/tests/InvalidXMLCharacter/Test.elm diff --git a/tests/ci.js b/tests/ci.js index 4cd7fa9f..f1db8616 100755 --- a/tests/ci.js +++ b/tests/ci.js @@ -292,6 +292,12 @@ describe('Testing elm-test on single Elm files', () => { } } + it(`Should not crash the junit reportor on invalid characters`, () => { + const itsPath = path.join('tests', 'InvalidXMLCharacter', 'Test.elm'); + const runResult = execElmTest([itsPath, '--report', 'junit'], cwd); + assertTestSuccess(runResult); + }); + it(`Should run every file in tests/CompileError`, () => { const filesFound = readdir(path.join(cwd, 'tests', 'CompileError')); assert.deepStrictEqual( diff --git a/tests/fixtures/tests/InvalidXMLCharacter/Test.elm b/tests/fixtures/tests/InvalidXMLCharacter/Test.elm new file mode 100644 index 00000000..d992c8ce --- /dev/null +++ b/tests/fixtures/tests/InvalidXMLCharacter/Test.elm @@ -0,0 +1,12 @@ +module InvalidXMLCharacter.Test exposing (invalidCharacter) + +import Expect +import Test exposing (..) + + +invalidCharacter : Test +invalidCharacter = + describe "The junit reporter should not crash due to invalid control characters" + [ test "backspace: \u{0008}" <| + \() -> Expect.pass + ] From 7f3e295c303a54e0e030d09921fef679ee4ff6b8 Mon Sep 17 00:00:00 2001 From: ilias Date: Tue, 14 Feb 2023 11:30:19 +0100 Subject: [PATCH 2/6] Encode invalid characters in XML --- lib/Supervisor.js | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/Supervisor.js b/lib/Supervisor.js index bc889142..e95ebf80 100644 --- a/lib/Supervisor.js +++ b/lib/Supervisor.js @@ -201,7 +201,23 @@ function run( xml.testsuite.testcase = xml.testsuite.testcase.concat(values); - console.log(XmlBuilder.create(xml).end()); + // The XmlBuilder by default does not remove characters that are + // invalid in XML, like backspaces. However, we can pass it an + // `invalidCharReplacement` option to tell it how to handle + // those characters, rather than crashing. In an attempt to + // retain useful information in the output, we try and output a + // hex-encoded unicode codepoint for the invalid character. For + // example, a backspace (`\u{08}` in Elm) will be output as + // `\0x8`. + var invalidCharReplacement = function (char) { + return '\\0x' + char.codePointAt(0).toString(16); + }; + + console.log( + XmlBuilder.create(xml, { + invalidCharReplacement: invalidCharReplacement, + }).end() + ); } } From 41e357c03a66b2ee872d5931d0c5640139a6b3aa Mon Sep 17 00:00:00 2001 From: ilias Date: Tue, 14 Feb 2023 12:01:01 +0100 Subject: [PATCH 3/6] Use Elm's format for unicode literals --- lib/Supervisor.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/Supervisor.js b/lib/Supervisor.js index e95ebf80..9f39ba63 100644 --- a/lib/Supervisor.js +++ b/lib/Supervisor.js @@ -207,10 +207,14 @@ function run( // those characters, rather than crashing. In an attempt to // retain useful information in the output, we try and output a // hex-encoded unicode codepoint for the invalid character. For - // example, a backspace (`\u{08}` in Elm) will be output as - // `\0x8`. + // example, a backspace (`\u{0008}` in Elm) will be output as a + // literal `\u{0008}`. var invalidCharReplacement = function (char) { - return '\\0x' + char.codePointAt(0).toString(16); + return ( + '\\u{' + + char.codePointAt(0).toString(16).padStart(4, '0') + + '}' + ); }; console.log( From df360236dd257a4a047aa28fdabcecea548a8379 Mon Sep 17 00:00:00 2001 From: ilias Date: Tue, 14 Feb 2023 16:03:54 +0100 Subject: [PATCH 4/6] Add extra test-case --- tests/fixtures/tests/InvalidXMLCharacter/Test.elm | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/fixtures/tests/InvalidXMLCharacter/Test.elm b/tests/fixtures/tests/InvalidXMLCharacter/Test.elm index d992c8ce..2da526e6 100644 --- a/tests/fixtures/tests/InvalidXMLCharacter/Test.elm +++ b/tests/fixtures/tests/InvalidXMLCharacter/Test.elm @@ -6,7 +6,9 @@ import Test exposing (..) invalidCharacter : Test invalidCharacter = - describe "The junit reporter should not crash due to invalid control characters" + describe "The junit reporter should not crash due to invalid (for XML) characters in the output" [ test "backspace: \u{0008}" <| \() -> Expect.pass + , test "escape: \u{001B}" <| + \() -> Expect.pass ] From 448eafc69fbff5fbb7ee844f15da1f7565c9a568 Mon Sep 17 00:00:00 2001 From: Ilias Van Peer <1038427+zwilias@users.noreply.github.com> Date: Tue, 14 Feb 2023 16:08:53 +0100 Subject: [PATCH 5/6] Update comment to reference terminal escapes as an example Seems like we're getting closer to a final colour for the bikeshed. Co-authored-by: Simon Lydell --- lib/Supervisor.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Supervisor.js b/lib/Supervisor.js index 9f39ba63..22ddbf2f 100644 --- a/lib/Supervisor.js +++ b/lib/Supervisor.js @@ -207,8 +207,8 @@ function run( // those characters, rather than crashing. In an attempt to // retain useful information in the output, we try and output a // hex-encoded unicode codepoint for the invalid character. For - // example, a backspace (`\u{0008}` in Elm) will be output as a - // literal `\u{0008}`. + // example, the start of a terminal escape (`\u{001B}` in Elm) will be output as a + // literal `\u{001B}`. var invalidCharReplacement = function (char) { return ( '\\u{' + From b6d5d8d86b872fe85af6cdf698ab19bd93921021 Mon Sep 17 00:00:00 2001 From: Ilias Van Peer <1038427+zwilias@users.noreply.github.com> Date: Tue, 14 Feb 2023 16:09:02 +0100 Subject: [PATCH 6/6] Fix typo Co-authored-by: Simon Lydell --- tests/ci.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ci.js b/tests/ci.js index f1db8616..3a16acbe 100755 --- a/tests/ci.js +++ b/tests/ci.js @@ -292,7 +292,7 @@ describe('Testing elm-test on single Elm files', () => { } } - it(`Should not crash the junit reportor on invalid characters`, () => { + it(`Should not crash the junit reporter on invalid characters`, () => { const itsPath = path.join('tests', 'InvalidXMLCharacter', 'Test.elm'); const runResult = execElmTest([itsPath, '--report', 'junit'], cwd); assertTestSuccess(runResult);