905340 - xpcshell test toolkit/identity/tests/unit/test_well-known.js fails on Android 4.0/Panda only

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Geoff Brown [:gbrown]]

Curiously, this seems to pass on Android 2.2 and the x86 emulator, but fails on Panda:

https://tbpl.mozilla.org/php/getParsedLog.php?id=26549350&tree=Try&full=1

11:35:35  WARNING -  TEST-UNEXPECTED-FAIL | /builds/panda-0822/test/build/tests/xpcshell/tests/toolkit/identity/tests/unit/test_well-known.js | test failed (with xpcshell return code: 0), see following log:
11:35:35     INFO -  >>>>>>>
11:35:35     INFO -  xpcw: cd /mnt/sdcard/tests/xpcshell/toolkit/identity/tests/unit
11:35:35     INFO -  xpcw: xpcshell -r /mnt/sdcard/tests/xpcshell/c/httpd.manifest --greomni /data/local/xpcb/fennec-26.0a1.en-US.android-arm.apk -m -n -s -e const _HTTPD_JS_PATH = "/mnt/sdcard/tests/xpcshell/c/httpd.js"; -e const _HEAD_JS_PATH = "/mnt/sdcard/tests/xpcshell/head.js"; -e const _TESTING_MODULES_DIR = "/mnt/sdcard/tests/xpcshell/m"; -f /mnt/sdcard/tests/xpcshell/head.js -e const _SERVER_ADDR = "localhost" -e const _HEAD_FILES = ["/mnt/sdcard/tests/xpcshell/toolkit/identity/tests/unit/head_identity.js"]; -e const _TAIL_FILES = ["/mnt/sdcard/tests/xpcshell/toolkit/identity/tests/unit/tail_identity.js"]; -e const _TEST_FILE = ["test_well-known.js"]; -e _execute_test(); quit(0);
11:35:35     INFO -  TEST-INFO | (xpcshell/head.js) | test MAIN run_test pending (1)
11:35:35     INFO -  TEST-INFO | (xpcshell/head.js) | test run_next_test 0 pending (2)
11:35:35     INFO -  TEST-INFO | (xpcshell/head.js) | test MAIN run_test finished (2)
11:35:35     INFO -  TEST-INFO | (xpcshell/head.js) | running event loop
11:35:35     INFO -  TEST-INFO | test_well-known.js | Starting test_well_known_1
11:35:35     INFO -  TEST-INFO | (xpcshell/head.js) | test test_well_known_1 pending (2)
11:35:35     INFO -  TEST-INFO | (xpcshell/head.js) | test pending (3)
11:35:35  WARNING -  TEST-UNEXPECTED-FAIL | (xpcshell/head.js) | TypeError: invalid 'in' operand error - See following stack:
11:35:35     INFO -  JS frame :: do_throw@/mnt/sdcard/tests/xpcshell/head.js:482
11:35:35     INFO -  JS frame :: _run_next_test@/mnt/sdcard/tests/xpcshell/head.js:1162
11:35:35     INFO -  JS frame :: do_execute_soon/<.run@/mnt/sdcard/tests/xpcshell/head.js:444
11:35:35     INFO -  TEST-INFO | (xpcshell/head.js) | exiting test
11:35:35     INFO -  TEST-INFO | (xpcshell/head.js) | test run_next_test 0 finished (3)
11:35:35     INFO -  Identity core: shutdown
11:35:35     INFO -  TEST-INFO | (xpcshell/head.js) | No (+ 0) checks actually run
11:35:35     INFO -  
11:35:35     INFO -  <<<<<<<

Comment 1

[Geoff Brown [:gbrown]]

:MattN -- You looked at identity xpcshell tests on Android in bug 875826. Now we are trying to run xpcshell tests on Android 4.0 for the first time and this one additional failure has come up. Should I just disable this test on Android, or does it need further investigation?

Comment 2

[Matthew N. [:MattN]]

I would say disable it for now on Android as nobody is currently working on this code and it's not shipped. Work will resume in the next few months and we can take a look then.

Comment 3

[Geoff Brown [:gbrown]]

Attachment: change server port from 8080 to 4444

I had a quick look at the failure anyway and noticed that server.start(8080) was failing. Most other tests that start a server specify a port of either -1 or 4444.

eg. http://hg.mozilla.org/mozilla-central/annotate/aada0f74faf9/toolkit/components/captivedetect/test/unit/test_captive_portal_found_303.js#l63

eg. http://hg.mozilla.org/mozilla-central/annotate/aada0f74faf9/toolkit/components/mediasniffer/test/unit/test_mediasniffer_ext.js#l112

Changing the port to 4444 gives a clean run on all of our test platforms:
https://tbpl.mozilla.org/?tree=Try&rev=ebcb7c662822&showall=1

Comment 4

[Matthew N. [:MattN]]

Comment on attachment 790792 [details] [diff] [review]
change server port from 8080 to 4444

(In reply to Geoff Brown [:gbrown] from comment #3)
> I had a quick look at the failure anyway and noticed that server.start(8080)
> was failing. Most other tests that start a server specify a port of either
> -1 or 4444.

-1 is the correct way nowadays.

> Changing the port to 4444 gives a clean run on all of our test platforms:
> https://tbpl.mozilla.org/?tree=Try&rev=ebcb7c662822&showall=1

Please use -1 and do like attachment 782224 [details] [diff] [review] to check the port so that this works in parallel with other tests. See bug 887064.

Comment 5

[Geoff Brown [:gbrown]]

Is this going to be okay:

-  // Change ports to avoid HTTP caching
-  SERVER_PORT++;
-  server.start(SERVER_PORT);
+  server.start(-1);

?

Comment 6

[Matthew N. [:MattN]]

(In reply to Geoff Brown [:gbrown] from comment #5)
> Is this going to be okay:
> -  // Change ports to avoid HTTP caching
> -  SERVER_PORT++;
> -  server.start(SERVER_PORT);
> +  server.start(-1);

You can do the following:

server.start(-1);
SERVER_PORT = server.identity.primaryPort;

I just had MDN updated to provide an example: https://developer.mozilla.org/en-US/docs/Httpd.js/HTTP_server_for_unit_tests#Running_the_server

Comment 7

[Geoff Brown [:gbrown]]

The docs look good - thanks.

My concern is that the existing test appears to be trying to guarantee that a different port is used for different parts of the test. I don't think that will be the case any more if we use -1.

Comment 8

[Geoff Brown [:gbrown]]

Sorry - nevermind! I see what you mean now.

Comment 9

[Geoff Brown [:gbrown]]

Attachment: change server port from 8080 to -1

Comment 10

[Matthew N. [:MattN]]

Comment on attachment 791049 [details] [diff] [review]
change server port from 8080 to -1

I'm worried that SERVER_PORT + 1 may be in use (especially now that tests are parallel). I guess I should have been more clear and stated I prefer to request a new random port for each test. I see what you're saying about guaranteeing a different port as it may not actually be random on some platforms. I'm not sure which one of these two intermittent failures would be more common as I don't know how many ports are chosen from when -1 is specified. I also believe it varies by OS.

We should really just fix the HTTP caching issue with HTTP headers so we don't need to change the port but like I said, we don't have people working on this code at the moment. That plus this patch would fix both problems. If you want to fix that too then you can add ^headers^ files for the two files being loaded. If the default 404 doesn't have headers to prevent caching then you can change it to be the last test as a quick workaround.

If you don't have time for that you can go ahead and disable this test on Android. The problem is that this seems like it will currently intermittently fail on desktop too.

Thanks for your time.

Comment 11

[Geoff Brown [:gbrown]]

Attachment: skip test_well-known.js on android

> If you don't have time for that you can go ahead and disable this test on Android.

Let's go with that then!

Comment 12

[Geoff Brown [:gbrown]]

Comment on attachment 791326 [details] [diff] [review]
skip test_well-known.js on android

Test disabled (skipped) on android in bug 865006.

Comment 13

[Justin Wood (:Callek)]

No longer using pandas at mozilla, but this hasn't had any action since 2013 anyway