Create View Pattern for use in Gaia Acceptance

RESOLVED WONTFIX

Status

defect
RESOLVED WONTFIX
4 years ago
4 years ago

People

(Reporter: Silne30, Assigned: Silne30)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

No description provided.
Working on base view for gaia integration PoC
QA Whiteboard: [fxosqa-auto-s12+]
Keywords: checkin-needed
Keywords: checkin-needed
QA Whiteboard: [fxosqa-auto-s12+] → [fxosqa-auto-s12+,s13]
Status: NEW → ASSIGNED
QA Whiteboard: [fxosqa-auto-s12+,s13] → [fxosqa-auto-s12+,s13,s14]
QA Whiteboard: [fxosqa-auto-s12+,s13,s14] → [fxosqa-auto-s12+,s13,s14,s15,s16,s17]
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Usually, when a fix happens, a link or a Pull Request should be attached to the bug (for tracking purpose). Otherwise, we consider the bug as "won't fix" or "duplicate" of a bug that changed the behavior.

John, would you mind updating the bug to reflect the correct state of the resolution?
Flags: needinfo?(jdorlus)
Sorry. Wrong bug.
Status: RESOLVED → REOPENED
Flags: needinfo?(jdorlus)
Resolution: FIXED → ---
Attachment #8680990 - Flags: review?(jlorenzo)
Comment on attachment 8680990 [details] [review]
[gaia] silne30:Bug_1145657_Create_View_Pattern_JS > mozilla-b2g:master

Great start! We can improve the readability and the intent behind this code. I left more details in the PR.
Attachment #8680990 - Flags: review?(jlorenzo)
Pushed changes in response to issues mentioned. There are some questions on the PR as well.
Flags: needinfo?(jlorenzo)
Flags: needinfo?(jlorenzo) → needinfo?(jdorlus)
[:jlorenzo]
Squashed my commits. Made fixes to the unnecessary waits.
Flags: needinfo?(jdorlus)
Attachment #8680990 - Flags: review?(jlorenzo)
Attachment #8680990 - Flags: review?(martijn.martijn)
Comment on attachment 8680990 [details] [review]
[gaia] silne30:Bug_1145657_Create_View_Pattern_JS > mozilla-b2g:master

Sorry, I don't know this code.
Attachment #8680990 - Flags: review?(martijn.martijn)
Comment on attachment 8680990 [details] [review]
[gaia] silne30:Bug_1145657_Create_View_Pattern_JS > mozilla-b2g:master

Johan told me that this should be pretty similar as Gaia UI test pattern.
Attachment #8680990 - Flags: review?(martijn.martijn)
Depends on: 1224301
Comment on attachment 8680990 [details] [review]
[gaia] silne30:Bug_1145657_Create_View_Pattern_JS > mozilla-b2g:master

I totally don't understand this. It all looks very confusing to me.
Why are we adding the tests here: apps/end2end/clock ?
Why not here: apps/clock/test/end2end/ ?
Attachment #8680990 - Flags: review?(martijn.martijn) → review+
Comment on attachment 8680990 [details] [review]
[gaia] silne30:Bug_1145657_Create_View_Pattern_JS > mozilla-b2g:master

Clearing review, because this test might change until Continuous Integration makes sure it remains valid. That pre-requisite is tracked under bug 1224301.

See bug 1224301 comment 3 for more details.
Attachment #8680990 - Flags: review?(jlorenzo)
I don't understand, this test can run on b2g desktop, no?
That's right. I forgot that technically, they can. If we do so, we'd require to create a new taskcluster job called "end-to-end".

Another issue if we go this way is that MarionetteJS doesn't allow to filter out the tests based on the capabilities of a target. This can be an issue the day we write device-only tests.

What do you guys think?
(In reply to Johan Lorenzo [:jlorenzo] (QA) - On PTO from Dec 23rd to Jan 3rd 2016 from comment #11)
> Comment on attachment 8680990 [details] [review]
> [gaia] silne30:Bug_1145657_Create_View_Pattern_JS > mozilla-b2g:master
> 
> Clearing review, because this test might change until Continuous Integration
> makes sure it remains valid. That pre-requisite is tracked under bug 1224301.
> 
> See bug 1224301 comment 3 for more details.

Since it seems that the decision was to keep this code in a PR, I think that this bug can be closed. No need keeping it opened since no further work will be done on it. If someone decides to reuse the views, they can access the PR. It seems that there are some big changes happening to the way MarionetteJS works based on what I saw in the flow chart. If that's the case, they may render these changes invalid. But in the meantime, I think this should be closed.
Flags: needinfo?(jlorenzo)
(In reply to John Dorlus [:Silne30] from comment #14)
> It seems that there are some big changes happening to the way
> MarionetteJS works based on what I saw in the flow chart.
I'm not too sure, but I thought that only the internals of MarionetteJS will be change (like fxos-device-service). Are also the WebDriver APIs and marionette-apps planned to be changed?

If not, I agree on the fact that the PR will likely not be 100% valid anymore when it lands. Nevertheless, I don't think the amount of changes will be huge. So, I'd keep the bug open until the code is actually merged in master. As the time frame is still uncertain, I propose to clear the assignee. What do you think, John?
Flags: needinfo?(jlorenzo)
Johan, I don't remember precisely, what prevents this code to be checked in? This should be possible to run in CI, no?
Flags: needinfo?(jlorenzo)
(In reply to Martijn Wargers [:mwargers] (QA) from comment #16)
> This should be possible to run in CI, no?

That might be the case. How do you think we can run it?
Flags: needinfo?(jlorenzo)
I don't think this will be of any value at this point. What do you think, Johan?
Flags: needinfo?(jlorenzo)
That's right. Closing this bug out as Firefox OS on smartphone is being sunset.
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Flags: needinfo?(jlorenzo)
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.