821866 - marAppApplyUpdateSuccess.js and marAppApplyUpdateStageSuccess.js failing on B2G due to 'Main application binary not found... expected: b2g-bin'

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Robert Strong (they/them - no direct email)]

+++ This bug was initially created as a clone of Bug #821343 +++

It is currently using the hardcoded value on "-bin" due to XP_UNIX being defined.

Comment 1

[Robert Strong (they/them - no direct email)]

Attachment: patch - use @BIN_SUFFIX@

Pushed to try
https://tbpl.mozilla.org/?tree=Try&rev=aa339fb77ca1&noignore=1

Note: there may be other failures after this failure.

Comment 2

[Robert Strong (they/them - no direct email)]

Made it a little further... now it is failing with

/home/cltbld/talos-slave/test/build/tests/xpcshell/tests/toolkit/mozapps/update/test/unit/test_0200_app_launch_apply_update.js | running test ...
19:21:18  WARNING -  TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/test/build/tests/xpcshell/tests/toolkit/mozapps/update/test/unit/test_0200_app_launch_apply_update.js | test failed (with xpcshell return code: 0), see following log:
19:21:18  WARNING -  This is a harness error.
19:21:18     INFO -  >>>>>>>
19:21:18     INFO -  xpcw: cd /data/local/tests/xpcshell/toolkit/mozapps/update/test/unit
19:21:18     INFO -  xpcw: xpcshell -r /data/local/tests/xpcshell/c/httpd.manifest -m -n -s -e const _HTTPD_JS_PATH = "/data/local/tests/xpcshell/c/httpd.js"; -e const _HEAD_JS_PATH = "/data/local/tests/xpcshell/head.js"; -e const _TESTING_MODULES_DIR = "/data/local/tests/xpcshell/m"; -f /data/local/tests/xpcshell/head.js -e const _SERVER_ADDR = "localhost" -e const _HEAD_FILES = ["/data/local/tests/xpcshell/toolkit/mozapps/update/test/unit/head_update.js"]; -e const _TAIL_FILES = []; -e const _TEST_FILE = ["test_0200_app_launch_apply_update.js"]; -e _execute_test(); quit(0);
19:21:18     INFO -  TEST-INFO | (xpcshell/head.js) | test 1 pending
19:21:18     INFO -  TEST-INFO | (xpcshell/head.js) | test 2 pending
19:21:18  WARNING -  TEST-UNEXPECTED-FAIL | xpcshell/head.js | [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIFile.create]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: test_0200_app_launch_apply_update.js :: run_test :: line 76"  data: no]
19:21:18  WARNING -  This is a harness error.
19:21:18     INFO -  test_0200_app_launch_apply_update.js:135: TypeError: gProcess is undefined

The failure occurs at
  if (!updateTestDir.exists()) {
    updateTestDir.create(AUS_Ci.nsIFile.DIRECTORY_TYPE, PERMS_DIRECTORY);
  }
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/test/unit/test_0200_app_launch_apply_update.js#76

Comment 3

[Robert Strong (they/them - no direct email)]

Also of interest after the above
/home/cltbld/talos-slave/test/build/tests/xpcshell/tests/toolkit/mozapps/update/test/unit/test_0201_app_launch_apply_update.js | running test ...
19:21:23  WARNING -  TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/test/build/tests/xpcshell/tests/toolkit/mozapps/update/test/unit/test_0201_app_launch_apply_update.js | test failed (with xpcshell return code: 0), see following log:
19:21:23  WARNING -  This is a harness error.
19:21:23     INFO -  >>>>>>>
19:21:23     INFO -  xpcw: cd /data/local/tests/xpcshell/toolkit/mozapps/update/test/unit
19:21:23     INFO -  xpcw: xpcshell -r /data/local/tests/xpcshell/c/httpd.manifest -m -n -s -e const _HTTPD_JS_PATH = "/data/local/tests/xpcshell/c/httpd.js"; -e const _HEAD_JS_PATH = "/data/local/tests/xpcshell/head.js"; -e const _TESTING_MODULES_DIR = "/data/local/tests/xpcshell/m"; -f /data/local/tests/xpcshell/head.js -e const _SERVER_ADDR = "localhost" -e const _HEAD_FILES = ["/data/local/tests/xpcshell/toolkit/mozapps/update/test/unit/head_update.js"]; -e const _TAIL_FILES = []; -e const _TEST_FILE = ["test_0201_app_launch_apply_update.js"]; -e _execute_test(); quit(0);
19:21:23     INFO -  TEST-INFO | (xpcshell/head.js) | test 1 pending
19:21:23     INFO -  TEST-INFO | (xpcshell/head.js) | test 2 pending
19:21:23     INFO -  Unable to remove files / directories from directory
19:21:23     INFO -  path: /data/local/updates
19:21:23     INFO -  Exception: [Exception... "Component returned failure code: 0x80520006 (NS_ERROR_FILE_TARGET_DOES_NOT_EXIST) [nsIFile.isDirectory]"  nsresult: "0x80520006 (NS_ERROR_FILE_TARGET_DOES_NOT_EXIST)"  location: "JS frame :: /data/local/tests/xpcshell/toolkit/mozapps/update/test/unit/head_update.js :: cleanUpdatesDir :: line 1068"  data: no]
19:21:23     INFO -  *** AUS:SVC readStatusFile - status: pending, path: /data/local/updates/0/update.status
19:21:23     INFO -  *** AUS:SVC UpdateManager:_loadXMLFileIntoArray: XML file does not exist
19:21:23     INFO -  TEST-PASS | test_0201_app_launch_apply_update.js | [run_test : 163] true == true
19:21:23  WARNING -  TEST-UNEXPECTED-FAIL | xpcshell/head.js | [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIFile.create]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: /data/local/tests/xpcshell/toolkit/mozapps/update/test/unit/head_update.js :: writeFile :: line 910"  data: no]
19:21:23  WARNING -  This is a harness error.
19:21:23     INFO -  test_0201_app_launch_apply_update.js:206: TypeError: gProcess is undefined

Comment 4

[Robert Strong (they/them - no direct email)]

I added a bunch of write access checks and this appears to be due to remounting as read after running the updater

06:17:55     INFO -  TEST-INFO | /home/cltbld/talos-slave/test/build/tests/xpcshell/tests/toolkit/mozapps/update/test/unit/test_0110_general.js | running test ...
06:18:03  WARNING -  TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/test/build/tests/xpcshell/tests/toolkit/mozapps/update/test/unit/test_0110_general.js | test failed (with xpcshell return code: 0), see following log:
...
06:18:03     INFO -  TEST-INFO | /data/local/tests/xpcshell/toolkit/mozapps/update/test/unit/head_update.js | [checkWriteAccess : 3087] testing write access in the application directory
06:18:03     INFO -  TEST-INFO | /data/local/tests/xpcshell/toolkit/mozapps/update/test/unit/head_update.js | [checkWriteAccess : 3091] create file - path: /system/b2g/update_file_test
06:18:03     INFO -  TEST-PASS | /data/local/tests/xpcshell/toolkit/mozapps/update/test/unit/head_update.js | [checkWriteAccess : 3093] true == true
06:18:03     INFO -  TEST-INFO | /data/local/tests/xpcshell/toolkit/mozapps/update/test/unit/head_update.js | [checkWriteAccess : 3094] remove file - path: /system/b2g/update_file_test
06:18:03     INFO -  TEST-PASS | /data/local/tests/xpcshell/toolkit/mozapps/update/test/unit/head_update.js | [checkWriteAccess : 3096] false == false
06:18:03     INFO -  TEST-INFO | /data/local/tests/xpcshell/toolkit/mozapps/update/test/unit/head_update.js | [checkWriteAccess : 3100] create directory - path: /system/b2g/update_dir_test
06:18:03     INFO -  TEST-PASS | /data/local/tests/xpcshell/toolkit/mozapps/update/test/unit/head_update.js | [checkWriteAccess : 3102] true == true
06:18:03     INFO -  TEST-INFO | /data/local/tests/xpcshell/toolkit/mozapps/update/test/unit/head_update.js | [checkWriteAccess : 3103] remove directory - path: /system/b2g/update_dir_test
06:18:03     INFO -  TEST-PASS | /data/local/tests/xpcshell/toolkit/mozapps/update/test/unit/head_update.js | [checkWriteAccess : 3105] false == false

...bunch of checks...

06:18:03     INFO -  TEST-INFO | /data/local/tests/xpcshell/toolkit/mozapps/update/test/unit/head_update.js | [checkWriteAccess : 3087] testing write access in the application directory
06:18:03     INFO -  TEST-INFO | /data/local/tests/xpcshell/toolkit/mozapps/update/test/unit/head_update.js | [checkWriteAccess : 3091] create file - path: /system/b2g/update_file_test
06:18:03  WARNING -  TEST-UNEXPECTED-FAIL | xpcshell/head.js | [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIFile.create]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: /data/local/tests/xpcshell/toolkit/mozapps/update/test/unit/head_update.js :: checkWriteAccess :: line 3092"  data: no]

06:18:03     INFO -  TEST-INFO | /home/cltbld/talos-slave/test/build/tests/xpcshell/tests/toolkit/mozapps/update/test/unit/test_0111_general.js | running test ...
06:18:08  WARNING -  TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/test/build/tests/xpcshell/tests/toolkit/mozapps/update/test/unit/test_0111_general.js | test failed (with xpcshell return code: 0), see following log:
...
06:18:08     INFO -  TEST-INFO | /data/local/tests/xpcshell/toolkit/mozapps/update/test/unit/head_update.js | [checkWriteAccess : 3087] testing write access in the application directory
06:18:08     INFO -  TEST-INFO | /data/local/tests/xpcshell/toolkit/mozapps/update/test/unit/head_update.js | [checkWriteAccess : 3091] create file - path: /system/b2g/update_file_test
06:18:08  WARNING -  TEST-UNEXPECTED-FAIL | xpcshell/head.js | [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIFile.create]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: /data/local/tests/xpcshell/toolkit/mozapps/update/test/unit/head_update.js :: checkWriteAccess :: line 3092"  data: no]

Comment 5

[Robert Strong (they/them - no direct email)]

Attachment: updater patch rev1

Comment 6

[Robert Strong (they/them - no direct email)]

Attachment: test patch rev1

Comment 7

[Robert Strong (they/them - no direct email)]

B2G try run of essentially these two patches with minor changes
https://tbpl.mozilla.org/?tree=Try&rev=588ab3724268

If all goes well I'll run a full try run over the weekend

Comment 8

[Robert Strong (they/them - no direct email)]

Comment on attachment 695037 [details] [diff] [review]
test patch rev1

The majority of the test changes are cleanup.

Comment 9

[Robert Strong (they/them - no direct email)]

Comment on attachment 695035 [details] [diff] [review]
updater patch rev1

I'll hold off on this until after I get try runs (hopefully before the end of the year ;)

Comment 10

[Brian R. Bondy [:bbondy]]

Comment on attachment 695037 [details] [diff] [review]
test patch rev1

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

::: toolkit/mozapps/update/test/unit/head_update.js.in
@@ +535,5 @@
>      env.set("MOZ_NO_REPLACE_FALLBACK", "1");
>    }
>  
> +  if (IS_ANDROID) {
> +    env.set("MOZ_PROCESS_UPDATES", "1");

Could you explain this change a bit more, I don't fully understand why it's needed.  Maybe just via a comment.

@@ +2202,5 @@
> +  logTestInfo("testing write access in the application directory");
> +  let testFile = getCurrentProcessDir();
> +  testFile.append("update_file_test");
> +  logTestInfo("create file - path: " + testFile.path);
> +  testFile.create(AUS_Ci.nsIFile.NORMAL_FILE_TYPE, 0644);

nit: would prefer createUnique in case something is in directory from a previous call

@@ +2211,5 @@
> +
> +  let testDir = getCurrentProcessDir();
> +  testDir.append("update_dir_test");
> +  logTestInfo("create directory - path: " + testDir.path);
> +  testDir.create(AUS_Ci.nsIFile.NORMAL_FILE_TYPE, 0755);

nit: createUnique

::: toolkit/mozapps/update/test/unit/test_0110_general.js
@@ +238,5 @@
>    do_test_pending();
>    do_register_cleanup(cleanupUpdaterTest);
>  
> +  // Test that B2G hasn't remounted as read-only after an update
> +  if (IS_ANDROID) {

is that the correct check for b2g? Maybe MOZ_WIDGET_GONK or MOZ_B2G_RIL?
Maybe this is right but I thought maybe this was for android mobile builds.

::: toolkit/mozapps/update/test/unit/test_0200_app_launch_apply_update.js
@@ -110,1 @@
>  

You are removing the creating of an ini file on purpose to avoid having a post update because this test is not focused on that? Is that correct?

@@ +129,5 @@
>    resetEnvironment();
>  }
>  
>  function end_test() {
> +  if (gProcess && gProcess.isRunning) {

Have noticed this when there were failures showing up in the logs, thanks for fixing here and elsewhere.

::: toolkit/mozapps/update/test/unit/test_0202_app_launch_apply_update_dirlocked.js
@@ -31,5 @@
>  // the test will try to kill it.
>  const APP_TIMER_TIMEOUT = 15000;
>  
>  let gAppTimer;
> -let gProcess;

While you're at it I think you can also remove gAppTimer here.

Comment 11

[Robert Strong (they/them - no direct email)]

(In reply to Brian R. Bondy [:bbondy] from comment #10)
> Comment on attachment 695037 [details] [diff] [review]
> test patch rev1
> 
> Review of attachment 695037 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/mozapps/update/test/unit/head_update.js.in
> @@ +535,5 @@
> >      env.set("MOZ_NO_REPLACE_FALLBACK", "1");
> >    }
> >  
> > +  if (IS_ANDROID) {
> > +    env.set("MOZ_PROCESS_UPDATES", "1");
> 
> Could you explain this change a bit more, I don't fully understand why it's
> needed.  Maybe just via a comment.
Sure. GONK remounts the partition as read only and the second patch makes a change to updater so it doesn't when running tests using the same env var used to exit early in nsAppRunner since it is also needed for those tests. Will add a comment.

> @@ +2202,5 @@
> > +  logTestInfo("testing write access in the application directory");
> > +  let testFile = getCurrentProcessDir();
> > +  testFile.append("update_file_test");
> > +  logTestInfo("create file - path: " + testFile.path);
> > +  testFile.create(AUS_Ci.nsIFile.NORMAL_FILE_TYPE, 0644);
> 
> nit: would prefer createUnique in case something is in directory from a
> previous call
Sure

> @@ +2211,5 @@
> > +
> > +  let testDir = getCurrentProcessDir();
> > +  testDir.append("update_dir_test");
> > +  logTestInfo("create directory - path: " + testDir.path);
> > +  testDir.create(AUS_Ci.nsIFile.NORMAL_FILE_TYPE, 0755);
> 
> nit: createUnique
> 
> ::: toolkit/mozapps/update/test/unit/test_0110_general.js
> @@ +238,5 @@
> >    do_test_pending();
> >    do_register_cleanup(cleanupUpdaterTest);
> >  
> > +  // Test that B2G hasn't remounted as read-only after an update
> > +  if (IS_ANDROID) {
> 
> is that the correct check for b2g? Maybe MOZ_WIDGET_GONK or MOZ_B2G_RIL?
> Maybe this is right but I thought maybe this was for android mobile builds.
I could split it out so there is an additional one specifically for GONK

> ::: toolkit/mozapps/update/test/unit/test_0200_app_launch_apply_update.js
> @@ -110,1 @@
> >  
> 
> You are removing the creating of an ini file on purpose to avoid having a
> post update because this test is not focused on that? Is that correct?
Correct

> toolkit/mozapps/update/test/unit/test_0202_app_launch_apply_update_dirlocked.
> js
> @@ -31,5 @@
> >  // the test will try to kill it.
> >  const APP_TIMER_TIMEOUT = 15000;
> >  
> >  let gAppTimer;
> > -let gProcess;
> 
> While you're at it I think you can also remove gAppTimer here.
Will check and remove it if it isn't used.

Comment 12

[Robert Strong (they/them - no direct email)]

Comment on attachment 695037 [details] [diff] [review]
test patch rev1

btw: there is a push to set flags after a review so it is possible to check how long it takes between request and review.

Comment 13

[Robert Strong (they/them - no direct email)]

(In reply to Robert Strong [:rstrong] (do not email) from comment #11)
> (In reply to Brian R. Bondy [:bbondy] from comment #10)
> > Comment on attachment 695037 [details] [diff] [review]
> > test patch rev1
> > 
> > Review of attachment 695037 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: toolkit/mozapps/update/test/unit/head_update.js.in
> > @@ +535,5 @@
> > >      env.set("MOZ_NO_REPLACE_FALLBACK", "1");
> > >    }
> > >  
> > > +  if (IS_ANDROID) {
> > > +    env.set("MOZ_PROCESS_UPDATES", "1");
> > 
> > Could you explain this change a bit more, I don't fully understand why it's
> > needed.  Maybe just via a comment.
> Sure. GONK remounts the partition as read only and the second patch makes a
> change to updater so it doesn't when running tests using the same env var
> used to exit early in nsAppRunner since it is also needed for those tests.
> Will add a comment.
Might go with its own env var as well

Comment 14

[Robert Strong (they/them - no direct email)]

Attachment: patch rev1

Pushed to try
https://treeherder.mozilla.org/#/jobs?repo=try&revision=199993102409

Comment 15

[Robert Strong (they/them - no direct email)]

Comment on attachment 8603747 [details] [diff] [review]
patch rev1

bah... still fails on b2g debug

Comment 16

[Robert Strong (they/them - no direct email)]

This test would have caught bug 1155704 so adding dependency

Comment 17

[Robert Strong (they/them - no direct email)]

Attachment: patch - only run on b2g opt

I'll filed bug 1163354 to try to get these tests to passing on b2g debug but I don't want to hold up getting the tests running on b2g opt.

Comment 18

[Robert Strong (they/them - no direct email)]

Try runs
Shows that the patch passes on all platforms except b2g debug for xpcshell tests
https://treeherder.mozilla.org/#/jobs?repo=try&revision=199993102409

Shows that disabling it on b2g debug passes on b2g opt and b2g debug for xpcshell tests
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a0d1290e5cf5

Comment 19

[Robert Strong (they/them - no direct email)]

All looks good with the try runs

Comment 20

[Robert Strong (they/them - no direct email)]

Couple of additional try runs just to be sure
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0424c84b53ab

Pushed to fx-team
https://hg.mozilla.org/integration/fx-team/rev/bde44865243f

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bde44865243f