Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add support for alternate Firefox application IDs, in particular Metro Firefox. #48

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants

This changes the application ID constant into an array to support multiple IDs.

Update dashboard.js
Add support for alternate Firefox application IDs, in particular Metro Firefox.

@davehunt davehunt commented on the diff Jan 13, 2013

dashboard.js
@@ -13,7 +13,8 @@ ddoc = {
ddoc.validate_doc_update = function(newDoc, oldDoc, userCtx) {
const MAX_SIZE = 1024 * 1024 * 10;
- const FIREFOX_APP_ID = "{ec8030f7-c20a-464f-9b0e-13a3a9e97384}";
+ const FIREFOX_APP_ID = {"{ec8030f7-c20a-464f-9b0e-13a3a9e97384}",
@davehunt

davehunt Jan 13, 2013

Member

This should be an array, for example:

const FIREFOX_APP_ID = ["{ec8030f7-c20a-464f-9b0e-13a3a9e97384}", "{99bceaaa-e3c6-48c1-b981-ef9b46b67d60}"];

@davehunt davehunt commented on the diff Jan 13, 2013

dashboard.js
@@ -51,7 +52,7 @@ ddoc.validate_doc_update = function(newDoc, oldDoc, userCtx) {
}
});
- if (newDoc.application_id !== FIREFOX_APP_ID) {
+ if (FIREFOX_APP_ID.indexOf(newDoc.application_id) == -1) {
@davehunt

davehunt Jan 13, 2013

Member

Use === instead of ==

Contributor

whimboo commented Jan 14, 2013

Also that's not enough. While this will pass the validity checks the dashboard will not be able to show the results. There is more work necessary here. At least in https://github.com/mozilla/mozmill-dashboard/blob/master/dashboard.js.

Member

davehunt commented Feb 15, 2013

What else would be needed Henrik? I think at least we would want to indicate when results are related to Metro mode. @ashughes1 is this something you're interested on working on? If not, perhaps you could provide some instructions for running the tests against Firefox in this mode so whomever picks this up can test their changes.

You can start Firefox with the Metro UI from the traditional desktop by passing the -metrodesktop command line argument. Be advised that the elements in the Metro UI are being reworked from the ground up, so I don't think there's going to be simple case of refactoring our tests to work in both modes. We'll likely need a whole new set of tests for Metro (a conversation I need to bring up with ctalbert and mfinkle).

All of that aside, adding support for multiple app IDs to the dashboard is something I would like to work on, but I'm not sure when I will find time.

Contributor

whimboo commented Feb 25, 2013

We would mix results from Firefox and Metro mode in the same type of application. We might have to implement application support to the dashboard. Not sure how useful that is given that we want to move to another system in the near future. We should discuss that in the automation meeting today.

Contributor

whimboo commented Mar 11, 2013

Given that we will have a separate testrun for metro tests we will also add a new entry for the dashboard. That way you can simply query for metro results. Means those will be isolated from all the other results.

Contributor

whimboo commented Mar 11, 2013

Anthony, shall we take that work over from you? If that's the case I would propose we close this pull for now and create a new one with our code.

Anthony, shall we take that work over from you? If that's the case I would propose
we close this pull for now and create a new one with our code.

Yes, Henrik, thank you. I don't think I will have time to work on this given my current priorities. Apologies for not responding earlier.

Contributor

whimboo commented Apr 15, 2013

The priority for this issue is pretty low at the moment. Way more important is to get the testrun type in place and being able to run those tests. See the following bug for more details:

https://bugzilla.mozilla.org/show_bug.cgi?id=845079

We will come back to this one once metro tests are working and we know what's necessary to implement.

Contributor

whimboo commented Apr 25, 2014

This already landed via PR #82.

@whimboo whimboo closed this Apr 25, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment