690493 - Proof of concept for OpenWebApps add-on test

Status: VERIFIED WONTFIX

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[Mohamed Dabbagh]

Attachment: JS File

The attached file contains a MozMill test that demos a proof of concept test run for automating the OpenWebApps extension/apps testing. It is only meant as a proof of concept and may not reflect what the final automation will be like. A proper test directory structure and proper tests will be created once the OWA extension works correctly and once there is a proper environment to run the tests in (apps, marketplace, etc). The test launches the sample apps page, installs an app, launches the app, closes it and then deletes it. Please follow the steps below to run the test and give it a try!

Steps to run the test:
1) Download the attached JS file and the xpi file as well
2) Make sure to have cloned mozmill-tests (if not please follow the steps found on https://developer.mozilla.org/en/Mozmill_Tests)
3) Place the JS file somewhere within the mozmill-tests folder
4) Place the xpi file anywhere
5) Edit the JS file and change the first two lines that state (...require = ) to point to the correct path of where lib is located
6) Launch mozmill and type: mozmill -t <path to test> --addons=<path to xpi file>

Comment 1

[Mohamed Dabbagh]

Attachment: Extension (xpi) file

Adding xpi file to be used with the test.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Mohammed, thanks for filing the bug. Before we can proceed here, please ensure that you create an add-ons test-run. We do not want to run the test with Mozmill core. Last week I have given you the link to the page on MDN. If it's unclear for you, I'm talking about a similar test as we have in '/tests/addons/[e-mail]'.

Also instead of attaching a file please create a single patch for any addition. Open questions we can sort out on IRC. Thanks.

Comment 3

[Mohamed Dabbagh]

Attachment: Patch version 1.0 - Added an openwebapps folder within the repo and created a demo test for OWA

Added a patch containing the owa demo test.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 563640 [details] [diff] [review]
Patch version 1.0 - Added an openwebapps folder within the repo and created a demo test for OWA

The patch looks good in general for demonstrating the feature set of the OWA extension and our add-ons testrun. I have some notes we should address before. Would be great if you can work on those earlier today, so that we have enough time for landing and the blog post. Thanks Mohamed!

>+[download]
>+linux=http://people.mozilla.com/~mdabbagh/openwebapps/extension/latest/openwebapps.xpi
>+mac=http://people.mozilla.com/~mdabbagh/openwebapps/extension/latest/openweba$
>+win=http://people.mozilla.com/~mdabbagh/openwebapps/extension/latest/openweba$

The last two URLs have somehow been cut-off. 

>+++ b/tests/addons/[e-mail]/tests/testOWADemo.js

Give it a real name like testInstallLaunchRemoveApp.js

>+ * The Initial Developer of the Original Code is Fidesfit
>+ * Portions created by the Initial Developer are Copyright (C) 2010

Both the year and the original code needs an update.

>+const GDELAY = 2500;

Thought about it and we shouldn't check it in that way. We only need the delay for the screencast. So leave it in for now, but please remove it for the final patch later once everything has been addressed.

>+var gPrefs = new Array("security.warn_entering_secure",
>+		       "security.warn_entering_weak",
>+		       "security.warn_leaving_secure",
>+		       "security.warn_submit_insecure",
>+		       "security.warn_viewing_mixed");
[..]
>+  for(var i = 0; i < gPrefs.length; i++)
>+    prefs.preferences.setPref(gPrefs[i], (i == 3));

We only need "security.warn_submit_insecure" here, so you can make it a simple constant.


>+function setupModule(module) {
>+  controller = mozmill.getBrowserController();
>+  
>+  tabBrowser = new tabs.tabBrowser(controller);
>+}
>+


>+  var installButton = new elementslib.XPath(controller.tabs.activeTab, "/html"+
>+                                            "/body/div[@id='wrapper']/section"+
>+                                            "[@id='projectContent']/div[@id="+
>+                                            "'contentWrap']/div[1]/div[1]");

Hint: you can directly start with contentWrap as first element in the Xpath string. But it's nothing worth experimenting yet.

>+  controller.waitThenClick(installButton, 8000, 100);
[..]
>+  controller.waitThenClick(notifInstallButton, 8000, 100);

Remove the latter 2 parameters. Those are not necessary.

>+  controller.keypress(new elementslib.XPath(controller.tabs.activeTab, 
>+                      "/html/body"), 'VK_ESCAPE', {});

I'm fairly sure it will also work when you do the keypress against the whole window. Simply specify null for the first parameter.

>+  // Checking if page is visible (only checking one element within the page)
>+  var appLoadedPic = new elementslib.XPath(controller.tabs.activeTab, 
>+                                           "/html/body[@id='body']/"+
>+                                           "center[2]/a/img");
>+  controller.waitForElement(appLoadedPic, 6000);

You will have to call waitForPageLoad() first for that tab. Then also remove the second parameter. In the tabBrowser class you can find a method which checks that the tab is an apptab. Please use the assertions module to assert that state.

>+  tabBrowser.closeTab("menu");

menu is used per default, so you can remove the parameter.

>+function testUninstallApp() {
>+  /* Not sure why but if this is not here, then the app is not 
>+     deleted but mozmill still says it passed? */
>+  controller.sleep(GDELAY);

It's a simple timing issue by switching tabs. The safest way would be to wait until the formerly selected tab is active again.

>+  controller.click(new elementslib.Link(controller.tabs.activeTab, "Delete"));

Please always declare the elements before you synthesize an action with it.

Comment 5

[Mohamed Dabbagh]

Attachment: Patch 2 - Second patch that contains fixes to the addon.ini file, renamed file and cleaned up code

Another patch that fixes the addon.ini file, renames the test file to testInstallLaunchRemoveApp.js and has cleaned up code as requested. There is still the problem with the part right before deleting the app where a sleep is needed. Multiple methods have been tried such as waiting for the tab, waiting for elements within the tab and selecting tab, but with no success. The sleep is needed for the demo anyways so for now it is sufficient. Will get fixed after the demo.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 563835 [details] [diff] [review]
Patch 2 - Second patch that contains fixes to the addon.ini file, renamed file and cleaned up code

As talked on IRC there were updates to the extension, which changed the behavior of installing apps. Also I have improved the test by using more already existent methods from our library. Given the dependency of all tests I moved the code into a single test function. I will upload a new version soon.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patch v3

Comment 8

[Mohamed Dabbagh]

Comment on attachment 563869 [details] [diff] [review]
Patch v3

Looks good/cleaner and runs exactly how it should. There is the problem with the about:apps page bug that will be fixed soon by checking the OS version that the test is running in. Good to go with demo!

Comment 9

[Mohamed Dabbagh]

Attachment: Patch v4 (demo)

Added an OS check to refresh the about:apps page if it's any platform other than Mac. If it's Mac, then a click on the page is performed instead. This is the only way that has been found to work fairly consistently across different platforms to avoid the bug where the apps list sometimes does not show in about:apps.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 563912 [details] [diff] [review]
Patch v4 (demo)

Looks good and works across platforms.

Renaming this patch because it will be used for the demo only. Mohamed, can you please remove all instances of the sleep call and the constant and check if it is still working? We would need such a stripped down patch for the final check-in. Thanks.

Comment 11

[Mohamed Dabbagh]

Attachment: Patch v5 - Demo

This patch just addresses the new change that involves using the new Dashboard. It is still for demo purposes. There are a couple of issues when running the test without sleeps and that is still being looked into. Once that is done, I will upload another patch that uses as little sleeps as possible without breaking the test.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 568257 [details] [diff] [review]
Patch v5 - Demo

>+	// Bypassing a security warning 
>+  prefs.preferences.setPref(PREF_SECURITY_WARN, false);
>+	// Launching the Dashboard to show no apps installed (demo purposes only)
>+  controller.open('https://myapps.mozillalabs.com/');

We should file a bug against the dashboard, if there is really a security warning. Seems like that they are loading content from an insecure page. Also can you make the URL a constant please? We use it multiple times in this test. And a little nit... please fix the indentation of the comment lines.

>+  // Opening the Dashboard to launch app
>+  controller.open('https://myapps.mozillalabs.com/');
>+  controller.waitForPageLoad();
>+
>+	// For demo purposes
>+	controller.sleep(DELAY_DEMO);
>+
>+	// Clicking on icon to launch app
>+	button = new elementslib.XPath(controller.tabs.activeTab, "//div[@id='apps']/div[2]/div" + 																															"[@id='page0']/div/div[1]/div");

Seems like the indentation is off a couple more times in this file. If it's a setting in your editor which broke it, please adjust.

>+  // Checking if tab is an app tab and is pinned
>+  assert.ok(tabBrowser.isAppTab(tabBrowser.getTab()),
>+             "Active tab has been pinned as AppTab");

I think that should be an expect and not assert use case. Even if it is not an app tab we want to continue the test.

>+  // Closing the app tab
>+  var tabCount = tabBrowser.length;
>+  tabBrowser.closeTab();
>+  controller.waitFor(function () {
>+    return tabBrowser.length === tabCount - 1;
>+  }, "The app tab was not closed properly");

Just call tabBrowser.closeTab() with the current tab object. It will take care of if the tab has been closed.

>+	controller.open('about:apps');
>+	controller.waitForPageLoad();

This has to be myapps.mozillalabs.com, or not? We are you using the internal dashboard now? I thought it has been abandoned?

>+  // Deleting the app
>+  var deleteAppButton = new elementslib.Link(controller.tabs.activeTab, "Delete");
>+  controller.waitThenClick(deleteAppButton);

Haven't we had another check on the dashboard that the app has been really removed?

Setting r- because a bit of code has been changed here and I would like to get some feedback and fixes.

Comment 13

[Mohamed Dabbagh]

(In reply to Henrik Skupin (:whimboo) from comment #12)
> Comment on attachment 568257 [details] [diff] [review] [diff] [details] [review]
> Patch v5 - Demo
> 
> >+	// Bypassing a security warning 
> >+  prefs.preferences.setPref(PREF_SECURITY_WARN, false);
> >+	// Launching the Dashboard to show no apps installed (demo purposes only)
> >+  controller.open('https://myapps.mozillalabs.com/');
> 
> We should file a bug against the dashboard, if there is really a security
> warning. Seems like that they are loading content from an insecure page.
> Also can you make the URL a constant please? We use it multiple times in
> this test. And a little nit... please fix the indentation of the comment
> lines.
> 
> >+  // Opening the Dashboard to launch app
> >+  controller.open('https://myapps.mozillalabs.com/');
> >+  controller.waitForPageLoad();
> >+
> >+	// For demo purposes
> >+	controller.sleep(DELAY_DEMO);
> >+
> >+	// Clicking on icon to launch app
> >+	button = new elementslib.XPath(controller.tabs.activeTab, "//div[@id='apps']/div[2]/div" + 																															"[@id='page0']/div/div[1]/div");
> 
> Seems like the indentation is off a couple more times in this file. If it's
> a setting in your editor which broke it, please adjust.
I apologize for this. I think I made an indentation change in my text editor recently that broke the indentation when making patches for some reason. It looks fine in my editor, which is probably why I didn't catch this.
> 
> >+  // Checking if tab is an app tab and is pinned
> >+  assert.ok(tabBrowser.isAppTab(tabBrowser.getTab()),
> >+             "Active tab has been pinned as AppTab");
> 
> I think that should be an expect and not assert use case. Even if it is not
> an app tab we want to continue the test.
So should I remove this part and just expect that the app is closed?
> 
> >+  // Closing the app tab
> >+  var tabCount = tabBrowser.length;
> >+  tabBrowser.closeTab();
> >+  controller.waitFor(function () {
> >+    return tabBrowser.length === tabCount - 1;
> >+  }, "The app tab was not closed properly");
> 
> Just call tabBrowser.closeTab() with the current tab object. It will take
> care of if the tab has been closed.
> 
> >+	controller.open('about:apps');
> >+	controller.waitForPageLoad();
> 
> This has to be myapps.mozillalabs.com, or not? We are you using the internal
> dashboard now? I thought it has been abandoned?
So the deal is that myapps.mozillalabs.com will display all the apps you have installed and you can launch them from there. However, it currently does not allow us to delete apps and the only way to delete apps is to go to about:apps. I wanted to use the Dashboard because this is the closest thing we have to a working Dashboard that is accessible through a website and not an 'about:*' page. It showcases what the real Dashboard might look like.

> 
> >+  // Deleting the app
> >+  var deleteAppButton = new elementslib.Link(controller.tabs.activeTab, "Delete");
> >+  controller.waitThenClick(deleteAppButton);
> 
> Haven't we had another check on the dashboard that the app has been really
> removed?
This is just actually deleting the app itself. The only place to delete the app is through about:apps and there is a 'Delete' button there that allows us to do so. 

> 
> Setting r- because a bit of code has been changed here and I would like to
> get some feedback and fixes.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Mohamed Dabbagh from comment #13)
> > >+  // Checking if tab is an app tab and is pinned
> > >+  assert.ok(tabBrowser.isAppTab(tabBrowser.getTab()),
> > >+             "Active tab has been pinned as AppTab");
> > 
> > I think that should be an expect and not assert use case. Even if it is not
> > an app tab we want to continue the test.
> So should I remove this part and just expect that the app is closed?

No, apps will have to be opened in app tabs, so we should have such a check. Just change it from assert to expect.

> have installed and you can launch them from there. However, it currently
> does not allow us to delete apps and the only way to delete apps is to go to
> about:apps. I wanted to use the Dashboard because this is the closest thing
> we have to a working Dashboard that is accessible through a website and not
> an 'about:*' page. It showcases what the real Dashboard might look like.

Ok, is the removal feature planned for the remote dashboard? If yes, can you point me to a bug? For now please add an XXX comment (similar to others in our repo) which states that.

> > Haven't we had another check on the dashboard that the app has been really
> > removed?
> This is just actually deleting the app itself. The only place to delete the
> app is through about:apps and there is a 'Delete' button there that allows
> us to do so. 

I mean that we should have one more check right after to ensure the app has really been deleted.

Comment 15

[Mohamed Dabbagh]

Attachment: Patch v6 - Demo

Re-worked the test to not use XPaths and made changes as requested by Henrik (simplified code, added extra checks, etc). This is still a demo test and so the sleeps are still required. This is because quite a few changes have been made and once those are approved I can work to make sure that it does not use sleeps for non-demo purposes.

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 569500 [details] [diff] [review]
Patch v6 - Demo

Mohamed, instead of nodeCollector please use elementslib.Selector to access nodes. nodeCollector doesn't really support content at the moment due to delayed loading of nodes. Such a change would also make the code way easier to read. I will reset the requests for now.

Comment 17

[Mohamed Dabbagh]

Attachment: Patch v7 - Demo

Edited the test to not use nodecollector but instead use selectors. This is still a demo so the sleeps are still there.

Comment 18

[Mohamed Dabbagh]

Attachment: Patch v8 - Test

Re-worked the test to not use sleeps anymore and removed the parts that relate to the demo run. Therefore, this patch is considered a test now and not a demo. I will not mark 'Patch 7 - Demo' as obsolete because that patch is for the demo only and this patch is for testing.

Comment 19

[Maniac Vlad Florin (:vladmaniac)]

Comment on attachment 569795 [details] [diff] [review]
Patch v7 - Demo


>+
>+var {expect} = require("../../../../lib/assertions");
>+var prefs = require("../../../../lib/prefs");
>+var tabs = require("../../../../lib/tabs");
>+var toolbars = require("../../../../lib/toolbars");
>+var domUtils = require("../../../../lib/dom-utils");

Please declare requirements in an alphabetical order, meaning only moving dom-utils up 

var {expect} = require("../../../../lib/assertions");
var domUtils = require("../../../../lib/dom-utils");
var prefs = require("../../../../lib/prefs");
var tabs = require("../../../../lib/tabs");
var toolbars = require("../../../../lib/toolbars");


>+
>+const DELAY_DEMO = 2500;
>+const PREF_SECURITY_WARN = "security.warn_viewing_mixed";
>+const DASHBOARD_URL = 'https://myapps.mozillalabs.com/';

Please refactor to 

const DASHBOARD_URL = 'https://myapps.mozillalabs.com/';
const PREF_SECURITY_WARN = "security.warn_viewing_mixed";

const DELAY_DEMO = 2500;

we are separating DELAY_DEMO from the rest because it's a constant marking a delay. we are doing that in all other tests
DASHBOARD_URL is first to keep alphabetical order

>+function setupModule(module) {
>+  module.controller = mozmill.getBrowserController();
>+
>+  module.tabBrowser = new tabs.tabBrowser(controller);
>+  module.locationBar = new toolbars.locationBar(module.controller);
>+
>+  module.tabBrowser.closeAllTabs();
>+}

For consistency, I would change 'module' to aModule - just like in other tests

>+  if(mozmill.isMac) {
>+    controller.click(new elementslib.XPath(controller.tabs.activeTab, "/html/body"));
>+  } else {
>+    controller.refresh();
>+    controller.waitForPageLoad();
>+  }
>+

Can we replace the XPath? I'm sure this is not a good practice


>+  // For demo purposes
>+  controller.sleep(DELAY_DEMO);
>+	
>+  // Deleting the app
>+  var deleteAppButton = new elementslib.Link(controller.tabs.activeTab, "Delete");

I would try to use Selector for the button instead of Link

Otherwise the demo looks fine.

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Thanks Vlad, but I think most of the feedback is not necessary for the demo script. Mohamed should take care of it for the final patch for sure. I will give my review soonish.

Comment 21

[Maniac Vlad Florin (:vladmaniac)]

(In reply to Henrik Skupin (:whimboo) from comment #20)
> Thanks Vlad, but I think most of the feedback is not necessary for the demo
> script. Mohamed should take care of it for the final patch for sure. I will
> give my review soonish.

Absolutely

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 569795 [details] [diff] [review]
Patch v7 - Demo

>+  controller.open('about:apps');
>+  controller.waitForPageLoad();
>+  /* about:apps is broken and doesn't display any content until you click
>+     somewhere onto the page
>+     Only workaround found was to check OS version and either click on page
>+     or refresh page */
>+  if(mozmill.isMac) {
>+    controller.click(new elementslib.XPath(controller.tabs.activeTab, "/html/body"));
>+  } else {
>+    controller.refresh();
>+    controller.waitForPageLoad();
>+  }

This is not working for me anymore on OS X. Mohamed, can you please check that on an OS X box? The about:apps page simply stays gray. :/ Can you please file a bug for it in the new extension component of the web apps project? Please cc me to it.

The code is ok for demo purposes, but it has to run. r- until we were able to make it work.

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 569850 [details] [diff] [review]
Patch v8 - Test

>+var domUtils = require("../../../../lib/dom-utils");

We can remove this line now after the nodeCollector usage has been removed.

>+const ABOUT_APPS_URL = 'about:apps';
>+const DASHBOARD_URL = 'https://myapps.mozillalabs.com/';
>+const SAMPLE_APPS_URL = 'https://apps.mozillalabs.com/appdir/';

nit: Can you use 'URL' as prefix instead as a suffix?

>+  // Bypassing a security warning 
>+  prefs.preferences.setPref(PREF_SECURITY_WARN, false);

This should be part of setupModule and also has to be reset in teardownModule.

>+  // Clicking on the install button of the first app in the sample apps page
>+  var installButton = new elementslib.Selector(controller.tabs.activeTab, "div.button.installable");
>+
>+  controller.waitThenClick(installButton);

nit: no need for an empty line. It should be a single block.

>+  // Clicking on the install button in the app install door hanger notification
>+  var doorHangerInstallButton = locationBar.getNotificationElement('openwebapps-install-notification-notification',
>+                                                                   '/anon([1])' +
>+                                                                   '/{"class":"popup-notification-button-container"}' +
>+                                                                   '/anon({"anonid":"button"})' +
>+                                                                   '/anon({"anonid":"button"})');

Can you please pre-declare the lookup string as a variable? That would make it way easier to read. Is it expected to have two "anonid":"button" lines?

>+  controller.waitThenClick(doorHangerInstallButton);

Same here with the empty line.

>+  // Clicking on the first app icon in the Dashboard to launch the app
>+  var dashboardAppIcon = new elementslib.Selector(controller.tabs.activeTab, "div.iconshader");
>+
>+  controller.waitForElement(dashboardAppIcon);
>+  tabBrowser.openInNewTab(dashboardAppIcon, "middleClick");
>+  controller.waitForPageLoad();

It doesn't work for me on OS X. No new tabs gets opened, instead I see a big red cross overlaying the icon.

>+  // Checking if tab is an app tab and is pinned
>+  expect.ok(tabBrowser.isAppTab(tabBrowser.getTab()),
>+            "Active tab has been pinned as AppTab");

Not "Active tab" but 'WebApp has been..."

>+  if(mozmill.isMac) {
>+    controller.click(new elementslib.XPath(controller.tabs.activeTab, "/html/body"));
>+  } else {
>+    controller.refresh();
>+    controller.waitForPageLoad();
>+  }

I assume I would see the same problems as for the demo here.

Looks good in general but we have to get the mentioned items fixed, before I can give a r+.

Comment 24

[Mohamed Dabbagh]

Attachment: Patch v9 - Test

Made the necessary changes in this patch as requested. I believe the problem with about:apps has to do with the build of the extension you are using. Can you please try with the extension here:

http://people.mozilla.org/~mdabbagh/openwebapps/extension/testing/openwebapps.xpi

I am able to view apps in the Dashboard on mac and can view about:apps with this extension build. If it still doesn't work for you, can you try manually going to myapps.mozillalabs.com and see if you can see the Dashboard with your installed apps? 

There is a slight problem with where we are getting the extension from as some branches in the openwebapps project contain code that is not found in the main (develop) branch. However, some of those branches have bugs in them related to the extension. I have filed a bug about one such case in bug 702391. I will need to talk with the openwebapps team to figure out which branch they are sticking with in terms of pushing working code so that we have one place to build the extension from.

This patch is for the actual test and not the demo. Once this test is given an r+ then it's just a matter of adding sleeps for the demo and will upload another patch for the blog/demo purposes.

Comment 25

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Mohamed Dabbagh from comment #24)

> Made the necessary changes in this patch as requested. I believe the problem
> with about:apps has to do with the build of the extension you are using. Can
> you please try with the extension here:
> 
> http://people.mozilla.org/~mdabbagh/openwebapps/extension/testing/
> openwebapps.xpi

I'm using the extension which is referenced in the patch. If that one is not correct, please update xpi on your people account accordingly. I thought it is always the latest code. Also this is the one and only XPI which will be used for those tests in your patch.

Comment 26

[Henrik Skupin [:whimboo][⌚️UTC+1]]

WTH, why is the latest version of the extension 45MB in size?

http://people.mozilla.com/~mdabbagh/openwebapps/extension/latest/

Comment 27

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 574481 [details] [diff] [review]
Patch v9 - Test

>+linux=http://people.mozilla.com/~mdabbagh/openwebapps/extension/latest/openwebapps.xpi
>+mac=http://people.mozilla.com/~mdabbagh/openwebapps/extension/latest/openwebapps.xpi
>+win=http://people.mozilla.com/~mdabbagh/openwebapps/extension/latest/openwebapps.xpi

I think that we should find a more public location of the XPI file, which wont be your people account where others don't have access to. Can you please talk with DavidC if you can upload it somewhere else?

>+  // Clicking on the first app icon in the Dashboard to launch the app
>+  var dashboardAppIcon = new elementslib.Selector(controller.tabs.activeTab, "div.iconshader");
>+  controller.waitForElement(dashboardAppIcon);
>+  tabBrowser.openInNewTab(dashboardAppIcon, "middleClick");

The issue here on OS X is the openInNewTab call, which does not open the app in a new tab. In any way the app is now visible in the dashboard with the latest XPI as given above.

I can't further execute this test for now. Mohamed, can you please get your patch tested on an OS X machine before submitting a new one?

Comment 28

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Mohamed, can you please give an update on this bug? What's the current state of testing of the patch on your side?

Comment 29

[Mohamed Dabbagh]

Sorry, I've been really busy with testing and supporting the apps team for their dev preview launch but I should be more free now since the code freeze is today. Anyhow, Apps is rapidly changing and so are the requirements and the UI. For example, now the door-hanger notification has 2 options to select and we can delete apps from the Dashboard now which means using another method to delete an app within our test will be needed to show the actual flow. I'll get working on this and will have it done by next week. Thanks!

Comment 30

[Jason Smith [:jsmith]]

The automation written here an approach re-evaluation. See rationale below.

With the move away from the extension soon to direct integration into Firefox 13 (https://wiki.mozilla.org/Web_Apps_integration), the add on extension test will become obsolete. The appdir page will not exist in the long term either.

Comment 31

[Jason Smith [:jsmith]]

Badly worded. Should be "The automation written here needs re-evaluation on the approach."

Comment 32

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Jason, it doesn't matter if the code is served via an extension or is included in Firefox proper. For the latter we simply wouldn't have to install the extension for the tests. Important to say is that the underlying API shouldn't change. Whenever the code is available in Firefox we would only have to move those tests to another test-run. I don't say that no code updates are necessary.

Comment 33

[Jason Smith [:jsmith]]

Henrik - There's significant changes and differences in what would be the proof of concept automation test for the web apps integration into desktop coming down the pipeline in FF 14 and higher. The extension will no longer be supported. I'll get you cc-ed on the discussion with Clint right now. The impression I'm getting however is that the requirements for a proof of concept will be different. For example, there's a lot of OS integration points that will be required to analyze native install, native uninstall, app launching, etc. 

I'd like to close this as a Won't Fix. Thoughts?

Comment 34

[Jason Smith [:jsmith]]

Note - I will open a new bug down the pipeline when the proof of concept is available on a per test basis, but I'd like to track that elsewhere.

Comment 35

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Jason, it's up to you how to proceed on this bug. I don't have the knowledge yet how all that stuff has been implemented meanwhile. If you think it's totally outdated, sure, go ahead and close it. I will have a look at the thread soon.

Comment 36

[Mohamed Dabbagh]

Closing as per Jason's comment #34. It is best as the functionality and requirements are constantly changing and the automation efforts are still being developed. Please add me to the new bug when opened. Thanks!