Closed Bug 1119957 Opened 5 years ago Closed 5 years ago

TEST-UNEXPECTED-FAIL | chat/components/src/test/test_logger.js | - "Hello, world!" == "Hello, world!Hello, world!"

Categories

(Chat Core :: General, defect)

x86_64
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mkmelin, Assigned: aleth)

References

Details

(Keywords: intermittent-failure, regression)

Attachments

(1 file)

https://treeherder.mozilla.org/ui/logviewer.html#?job_id=7007&repo=comm-central

WARNING - TEST-UNEXPECTED-FAIL | chat/components/src/test/test_logger.js | - "Hello, world!" == "Hello, world!Hello, world!"
04:57:41 INFO - /builds/slave/test/build/tests/xpcshell/tests/chat/components/src/test/test_logger.js:test_appendToFile:265
04:57:41 INFO - self-hosted:InterpretGeneratorResume:659
04:57:41 INFO - self-hosted:next:585
04:57:41 INFO - _run_next_test@/builds/slave/test/build/tests/xpcshell/head.js:1347:9
04:57:41 INFO - do_execute_soon/<.run@/builds/slave/test/build/tests/xpcshell/head.js:616:9
04:57:41 INFO - _do_main@/builds/slave/test/build/tests/xpcshell/head.js:184:5
04:57:41 INFO - _execute_test@/builds/slave/test/build/tests/xpcshell/head.js:476:5
04:57:41 INFO - @-e:1:1
04:57:41 INFO - exiting test
04:57:41 INFO - Unexpected exception 2147500036
04:57:41 INFO - undefined
04:57:41 INFO - exiting test
04:57:41 INFO - <<<<<<<
Component: Instant Messaging → General
Product: Thunderbird → Chat Core
Version: 31 → trunk
This fixes the test failure, but I don't yet understand exactly what's causing it.
appendToFile(aPath, aEncodedString) creates a Task with a reference to aEncodedString. When the second Task is run (but not when it is created) aEncodedString is an empty Uint8Array object. The question is why.
Unless I'm missing something obvious (hence the f?), this may need a bisect of m-c commits to figure out.
Attachment #8546888 - Flags: feedback?(clokep)
Attachment #8546888 - Flags: feedback?(nhnt11)
Prime suspect is bug 1077354.
Attachment #8546888 - Flags: feedback?(nhnt11)
Attachment #8546888 - Flags: feedback?(clokep)
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Depends on: 1077354
Comment on attachment 8546888 [details] [diff] [review]
loggertestfix.diff

From a discussion on #jsapi, it seems the OS.File behaviour change (ArrayBuffer parameters get emptied by OS.File calls) is here to stay, so we need to land this fix.
Attachment #8546888 - Flags: review?(clokep)
Comment on attachment 8546888 [details] [diff] [review]
loggertestfix.diff

Review of attachment 8546888 [details] [diff] [review]:
-----------------------------------------------------------------

I got a bit more explanation over IRC:

8:12:48 AM - clokep_work: aleth: Can you walk me through the changes in bug 1119957?
8:12:51 AM - instantbot: Bug https://bugzilla.mozilla.org/show_bug.cgi?id=1119957 nor, --, ---, aleth, ASSI, TEST-UNEXPECTED-FAIL | chat/components/src/test/test_logger.js | - "Hello, world!" == "Hello, world!
8:13:01 AM - clokep_work: The first hunk just adds a let...that's not a big deal.
8:13:38 AM - aleth: ArrayBufferViews passed as an argument to OS.File.write now get emptied by that call, so you can't reuse them
8:15:54 AM - aleth: https://bugzilla.mozilla.org/show_bug.cgi?id=1077354#c33
8:15:56 AM - instantbot: Bug 1077354 nor, --, mozilla37, jcoppeard, RESO FIXED, [OS.File] Replace C pointer trickery with proper ArrayBuffer transfers
8:21:09 AM - clokep_work: aleth: OK...so you're creating a new ArrayBuffer view before calling the method now?
8:21:18 AM - clokep_work: I'm not entirely sure what TextEncoder does?
8:21:40 AM - clokep_work: Are we confident that this is just a problem with the tests? Not a real problem?
8:22:32 AM - aleth: Yes, at least afaik we don't reuse any buffers in the logger code

::: chat/components/src/test/test_logger.js
@@ +255,5 @@
>  
>  let test_appendToFile = function* () {
>    const kStringToWrite = "Hello, world!";
>    let path = OS.Path.join(OS.Constants.Path.profileDir, "testFile.txt");
> +  let textencoder = new TextEncoder();

Stupid nit, but textEncoder or just encoder.
Attachment #8546888 - Flags: review?(clokep) → review+
Nit fixed on checkin.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.6
You need to log in before you can comment on or make changes to this bug.