Closed
Bug 1466899
Opened 6 years ago
Closed 6 years ago
TPS - Tabs: allow profile property be optional or at least match variable profile
Categories
(Firefox :: Sync, defect, P1)
Firefox
Sync
Tracking
()
RESOLVED
FIXED
Firefox 63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: isabel_rios, Assigned: markh)
References
(Depends on 1 open bug)
Details
Attachments
(2 files, 1 obsolete file)
Trying to implement an integration test which opens a tab on firefox-ios and checks that it is open on desktop is giving an error. We are facing an issue with the profile match. The test fails in the assertion because the profile is different depending on the environment the test runs. Would it be possible to make this property optional or at least allow a fuzzy match like "Fennec on *"? This is how we are defining the test_tabs.js: EnableEngines(["tabs"]); var phases = { "phase1": "profile1" }; // expected tabs state var tabs1 = [ { uri: "www.mozilla.org", profile: "profile1" } ]; // sync and verify tabs Phase("phase1", [ [Sync], [Tabs.verify, tabs1] ]); But the profile changes for each environment so, if we change the profile to the environment's owner profile, then the test passes. If changing the property to optional or allowing a fuzzy match is not allowed, is there any workaround we can follow to get the test working? Thanks
Assignee | ||
Comment 1•6 years ago
|
||
From a quick look, the "profile" entry in a tabs array is probably better named something like "clientName", so yeah, using a regex or similar would probably work fine. However, it seems like it would be more valuable if you could ensure the device name for the iOS device was fully under your control for a future where multiple iOS or Android devices could be involved in a single test. If you did that, you probably wouldn't need a change here as you could just make the iOS device be named "profile1".
Comment 2•6 years ago
|
||
(In reply to Mark Hammond [:markh] from comment #1) > From a quick look, the "profile" entry in a tabs array is probably better > named something like "clientName", so yeah, using a regex or similar would > probably work fine. However, it seems like it would be more valuable if you > could ensure the device name for the iOS device was fully under your control > for a future where multiple iOS or Android devices could be involved in a > single test. If you did that, you probably wouldn't need a change here as > you could just make the iOS device be named "profile1". Thanks Mark, you make a great point. Isabel is looking into setting the iOS client name via the UI to see if that works, though I believe bug 1466940 will make testing this a little easier. If this does work, we can look into alternative ways of setting the client name to avoid uneccesary UI interactions.
Assignee | ||
Comment 3•6 years ago
|
||
I think that the following patch should work - it should check the correct client name rather than the bad one stored in the tabs collection. Let me know if it does!
Comment 4•6 years ago
|
||
Thanks Mark. Isabel can you test this patch? Let me know if you need any help.
Flags: needinfo?(irios.mozilla)
Reporter | ||
Comment 5•6 years ago
|
||
Thanks Mark. Sure Dave! I will test this patch and let you both know.
Flags: needinfo?(irios.mozilla)
Reporter | ||
Comment 6•6 years ago
|
||
Hi Mark, This is working fine!\o/ thanks for the patch. Dave, so far I only got it working if I set the device's name via UI. Using a launchArgument, I see the device's name changed but the change is not seen on desktop so the test fails. I keep looking into that while this patch lands. Thank you for your help!
Updated•6 years ago
|
Priority: -- → P1
Assignee | ||
Updated•6 years ago
|
Attachment #8985710 -
Flags: review?(tchiovoloni)
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → markh
Status: NEW → ASSIGNED
Comment 7•6 years ago
|
||
Comment on attachment 8985710 [details] [diff] [review] tabs.patch Review of attachment 8985710 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me!
Attachment #8985710 -
Flags: review?(tchiovoloni) → review+
Assignee | ||
Updated•6 years ago
|
Keywords: checkin-needed
Pushed by dluca@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/69a5a04f2906 TPS - Tabs: allow profile property be optional or at least match variable profile. r=tchiovoloni
Keywords: checkin-needed
Comment 9•6 years ago
|
||
Backed out changeset 69a5a04f2906 (bug 1466899) for ESlint failure at services/sync/tps/extensions/tps/resource/modules/tabs.jsm Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/b6f1b6284f474a08953cd3b58709c14c5cd5e684 Failure push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=86c39d3d96f91334345398c2f3d74193a09e3d66&selectedJob=183908219 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=183908219&repo=mozilla-inbound&lineNumber=258 [task 2018-06-20T02:25:58.068Z] Installing setuptools, pip, wheel...done. [task 2018-06-20T02:25:59.169Z] running build_ext [task 2018-06-20T02:25:59.169Z] building 'psutil._psutil_linux' extension [task 2018-06-20T02:25:59.169Z] creating build [task 2018-06-20T02:25:59.169Z] creating build/temp.linux-x86_64-2.7 [task 2018-06-20T02:25:59.169Z] creating build/temp.linux-x86_64-2.7/psutil [task 2018-06-20T02:25:59.169Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_common.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_common.o [task 2018-06-20T02:25:59.169Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_posix.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o [task 2018-06-20T02:25:59.169Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_linux.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_linux.o [task 2018-06-20T02:25:59.169Z] creating build/lib.linux-x86_64-2.7 [task 2018-06-20T02:25:59.169Z] creating build/lib.linux-x86_64-2.7/psutil [task 2018-06-20T02:25:59.169Z] x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wl,-Bsymbolic-functions -Wl,-z,relro -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/psutil/_psutil_common.o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o build/temp.linux-x86_64-2.7/psutil/_psutil_linux.o -o build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so [task 2018-06-20T02:25:59.169Z] building 'psutil._psutil_posix' extension [task 2018-06-20T02:25:59.169Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_common.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_common.o [task 2018-06-20T02:25:59.169Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_posix.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o [task 2018-06-20T02:25:59.169Z] x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wl,-Bsymbolic-functions -Wl,-z,relro -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/psutil/_psutil_common.o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o -o build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so [task 2018-06-20T02:25:59.169Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so -> psutil [task 2018-06-20T02:25:59.169Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so -> psutil [task 2018-06-20T02:25:59.169Z] [task 2018-06-20T02:25:59.169Z] Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt) [task 2018-06-20T02:31:45.863Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/services/sync/tps/extensions/tps/resource/modules/tabs.jsm:75:57 | Missing semicolon. (semi) [taskcluster 2018-06-20 02:31:46.253Z] === Task Finished === [taskcluster 2018-06-20 02:31:46.253Z] Unsuccessful task run with exit code: 1 completed in 607.663 seconds
Flags: needinfo?(markh)
Assignee | ||
Updated•6 years ago
|
Attachment #8985710 -
Attachment is obsolete: true
Flags: needinfo?(markh)
Assignee | ||
Comment 10•6 years ago
|
||
Fixes eslint failure
Comment 12•6 years ago
|
||
Tried to land the patch but I got the following conflict: mozilla@ubuntu:~/mozilla-unified$ hg qimport bz://1466899 Fetching... done Parsing... done adding 1466899 to series file renamed 1466899 -> t.patch mozilla@ubuntu:~/mozilla-unified$ hg qseries t.patch mozilla@ubuntu:~/mozilla-unified$ hg qpush -a applying t.patch unable to find 'sync/tps/extensions/tps/resource/modules/tabs.jsm' for patching (use '--prefix' to apply patch relative to the current directory) 1 out of 1 hunks FAILED -- saving rejects to file sync/tps/extensions/tps/resource/modules/tabs.jsm.rej patch failed, unable to continue (try -v) patch failed, rejects left in working directory errors during apply, please fix and qrefresh t.patch
Flags: needinfo?(markh)
Updated•6 years ago
|
Keywords: checkin-needed
Comment 13•6 years ago
|
||
MozReview-Commit-ID: K9Vep0k2nRF
Comment 14•6 years ago
|
||
Comment on attachment 8987703 [details] Bug 1466899 - TPS now checks the name from the clients collection rather than the tabs collection. r=tchiovoloni Thom Chiovoloni [:tcsc] has approved the revision. https://phabricator.services.mozilla.com/D1819
Attachment #8987703 -
Flags: review+
Comment 15•6 years ago
|
||
Pushed by mhammond@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/40eebd227af7 TPS now checks the name from the clients collection rather than the tabs collection. r=tcsc
Comment 16•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/40eebd227af7
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(markh)
You need to log in
before you can comment on or make changes to this bug.
Description
•