The default bug view has changed. See this FAQ.

Add a testing infrastructure to test stuff related to Web Apps in mochitests

RESOLVED FIXED in mozilla17

Status

()

Core
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mounir, Assigned: mounir)

Tracking

Trunk
mozilla17
Points:
---

Firefox Tracking Flags

(blocking-kilimanjaro:+, blocking-basecamp:+)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

5 years ago
The code behaving differently for Web Apps grows everyday but we still have no way to test it except manual testing with Gaia.
We should add tests in m-c for that. We will need to create fake webapps in the test profile.
It would be really nice if we could test from mochitest-plain.  I don't know if that's feasible.
At some points etienne and I have write some tests based on mochitests for Gaia, but they didn't gain much traction and they have been... sorry really... abandonned.
(Assignee)

Comment 3

5 years ago
I had something somewhat working yesterday. However, I'm not able to get Fullscreen working with that but for some reasons, nsGlobalWindow::mApp is never set (if gdb doesn't lie to me) even if the correct app is found. I will have a deeper look at that today.
(Assignee)

Updated

5 years ago
Assignee: nobody → mounir
(Assignee)

Comment 4

5 years ago
Created attachment 639329 [details] [diff] [review]
Patch

We can easily add new apps in the test profile.
Using an app in a test requires you to have the mozbrowser privileges (should we make that available by default for say app.example.org?) and set @mozapp to a valid manifest URL. For the moment, the content of the frame can even be in another origin, it will still be flagged as installed content. That might change though.
Attachment #639329 - Flags: review?(khuey)
(Assignee)

Comment 5

5 years ago
Created attachment 639330 [details] [diff] [review]
Patch

I didn't meant to have two apps for the moment. That was only done for testing purposes (making sure the generated json is correct).
Attachment #639329 - Attachment is obsolete: true
Attachment #639329 - Flags: review?(khuey)
Attachment #639330 - Flags: review?(khuey)
Comment on attachment 639330 [details] [diff] [review]
Patch

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

I'm not a test harness peer.
Attachment #639330 - Flags: review?(khuey) → review?(ted.mielczarek)
I'm going to need two apps for my tests.  I can add that as a follow-up patch if you'd like.
(Assignee)

Comment 8

5 years ago
Created attachment 642466 [details] [diff] [review]
Patch

I completely forgot that I had another patch with quite more applications (I'm using those in another patch in my patch queue).
Attachment #639330 - Attachment is obsolete: true
Attachment #639330 - Flags: review?(ted.mielczarek)
Attachment #642466 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 9

5 years ago
This should be a blocker because we need to test a lot of stuff based on apps. The security model is maybe the biggest one.
Status: NEW → ASSIGNED
blocking-basecamp: --- → ?
(Assignee)

Updated

5 years ago
Blocks: 758258
Okay, so I'm confused a bit. A while back I know David Clarke ported over a bunch of mochitests for the mozapps API that should be in m-c right now. I know there's been some check-ins to it post the implementation as well (it's actively maintained). So what's different here? What are you trying to do that's different? And can the existing tests be reused? Why or Why Not?
(In reply to Jason Smith [:jsmith] from comment #10)
> Okay, so I'm confused a bit. A while back I know David Clarke ported over a
> bunch of mochitests for the mozapps API that should be in m-c right now. I
> know there's been some check-ins to it post the implementation as well (it's
> actively maintained). So what's different here? What are you trying to do
> that's different? And can the existing tests be reused? Why or Why Not?

There's also work I've seen on the Marionette side of the world in regards to apps (I know I saw a perf test for launching an app somewhere in the gaia repo...). Can someone clarify the differences here and why they are needed?
(Assignee)

Updated

5 years ago
Attachment #642466 - Flags: review?(jmaher)
(Assignee)

Updated

5 years ago
Attachment #642466 - Flags: review?(ctalbert)

Updated

5 years ago
Attachment #642466 - Flags: review?(jmaher) → review+
(Assignee)

Comment 12

5 years ago
Comment on attachment 642466 [details] [diff] [review]
Patch

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

Thank you for the fast review :)
Attachment #642466 - Flags: review?(ted.mielczarek)
Attachment #642466 - Flags: review?(ctalbert)
blocking-basecamp: ? → +
blocking-kilimanjaro: --- → +

Comment 13

5 years ago
(In reply to Jason Smith [:jsmith] from comment #10)
> Okay, so I'm confused a bit. A while back I know David Clarke ported over a
> bunch of mochitests for the mozapps API that should be in m-c right now. I
> know there's been some check-ins to it post the implementation as well (it's
> actively maintained). So what's different here? What are you trying to do
> that's different? And can the existing tests be reused? Why or Why Not?

I'll take a stab at answering this.
I don't know exactly what dclarke's mochitests do w.r.t. apps. The framework here being used inside mochitest came after that and allows mochitest to run the app runtime. This is needed because developers need a way to test their patches to the web app code in the short term. At the time we designed this hack atop mochitest (last February) we didn't know when Marionette would land,and the apps team thought they were landing sooner. So we pushed through. 

We have a more complete and far simpler Marionette based apps testing framework that is being developed as well. This will have more ability to verify attributes of an installed app that QA cares about and will be able to work with real end-user apps rather than fake apps like mochitest can use (of course the marionette based framework can also use these fake apps too). It is my hope that we move the web apps testing system to the marionette based framework (you shouldn't have to change any tests) and we remove these hacks from mochitest once we have the new system in place. But, that is how this came to be, and why we have two implementations here - one is the short term solution and one is the longer term solution.
Note that we implemented two test suites: one which was the framework to test the webapp runtime.  But we also implemented mochitests for the mozApps API, see bug 741549

Not sure how they overlap here with what you're trying to test
If I understand what Clint is saying, this implementation is intending to allow you to run mochitests within a web app on B2G, correct? If that's correct, then yeah, this is different the mozapps API mochitests and the desktop runtime mochitests.
(Assignee)

Comment 16

5 years ago
It's quite different: we want to be able to tests mozapps so having pre-installed apps in the profile. But it seems like tests in bug 741549 are breaking this :(
how are those tests breaking this? can you explain more?
(Assignee)

Comment 18

5 years ago
Justin is having a look at that. He might know better.
Right now, it's failing to uninstall the apps Mounir pre-installs.  I'm not sure why it's failing, yet.

That the first step of this test is to uninstall all apps in the profile seems pretty bogus to me.  :-/
I'd ask on IRC, but it's down for me:

The webapps code identifies an app to uninstall by its origin (Webapps.jsm, uninstall function).  But looking at the apps defined in automation.py.in, we have multiple apps with the same origin.  So...that does not seem right, and is likely part of the problem.
Created attachment 643945 [details] [diff] [review]
Followup: Don't use "/" inside app names, and don't declare multiple apps with the same origin.
Attachment #643945 - Flags: review?(mounir)
(Assignee)

Comment 22

5 years ago
Comment on attachment 643945 [details] [diff] [review]
Followup: Don't use "/" inside app names, and don't declare multiple apps with the same origin.

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

Thanks Justin.
Attachment #643945 - Flags: review?(mounir) → review+
(Assignee)

Updated

5 years ago
Duplicate of this bug: 768334
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa4ec4d3802a
https://hg.mozilla.org/integration/mozilla-inbound/rev/795c96108226
Followup:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fe71c51ee0b9
https://hg.mozilla.org/mozilla-central/rev/aa4ec4d3802a
https://hg.mozilla.org/mozilla-central/rev/795c96108226
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.