mozilla :: #testpilot

19 Apr 2017
16:08ckpricema's house wifi, let me know if i have any action
16:09clouserwam I the only one with crazy robot voices?
16:17* clouserw restarts computer
16:17JSON_voorheesMore evidence that discourse might not be the best place for updates, The queue release post from 5 days ago still has zero views... :(
16:18JSON_voorheesoh excuse me, 32 views, 0 replies
16:18clouserwnews feed is coming!
16:22fzzzyhmm, partial page refreshes might be trivial in the remove-react-router branch. i think all that is needed is to load the appropriate page bundle. (so if on experiments.html, just create a script tag, set the source to home.js, and put it in the dom to switch from the experiment page to the home page).
16:22fzzzythere'd still be the problem with csp and pushState though so have to figure out what is going on there
16:23chuckIf y'all want to see some sweet SQL, check out the sankey chart I made to show visualizations of the Pulse lifecycle:
16:24fzzzychuck: wow that is totally awesome, what am i looking at?
16:24chuckThere are some weird data irregularities, likely due to timestamp imprecision, but the majority paths are clear.
16:24chuckIt illustrates the frequencies with which users go through the Pulse reporting funnel.
16:24chuckTwo primary entry points: they are prompted, or they manually load the survey.
16:25mreidchuck: that's rad!
16:26chuckThen you can see how about 60% of the time users manually load the survey, they immediately close it. The remainder of the time they end up submitting.
16:26fzzzyI didn't know they could manually load it, that's good
16:26chuckManually load through the pageAction, or prompted via heartbeat-style thing.
16:27chuckIt loads the same survey in both cases.
16:28chuckIf you look at the table rather than the chart, it's basically just the frequency that events in the same lifecycle happen in which order.
17:13aswanjgruen, clouserw: in that screenshot john is showing, is test pilot installed with mozAddonmanager?
17:13jgruenaswan: yeah
17:13jgrueni can do a live walkthrough
17:14aswanokay, the intention is that the Promise returned from install() shouldn't resolve until that dialog is dismissed. so if you wait for that, you should be able to avoid the stacked notifications
17:16fzzzypretty sure we wait for the promise...
17:17fzzzywhat dialog is showing? i think we only have a dialog on install on the non-mozAddonManager codepath
17:18aswanfzzzy: john was showing a screenshot where the post-install notification for test pilot was showing at the same time as a permissions prompt for something else (snooze tabs mabye?)
17:18aswanthe point was the stacked notifications though
17:19fzzzyjgruen: where's the screenshot?
17:19aswanhe's talking in this other meeting at the moment :-)
17:20fzzzyheh ok
17:22fzzzyi can get you details about what exactly the code is doing
17:24jgruenlast screenshot
17:25jgruenonly happens if you one click install txp + experiment
17:28fzzzyit looks like that is a new thing in 57? ok, that's why i hadn't seen it before
17:28fzzzyI used to use devedition
17:28aswanthe lousy thing about waiting would be that the download of the second extension wouldn't start until after the notification for test pilot is dismissed. bug 1329884 is about addressing that
17:28firebot NEW, Handle notifications for simultaneous installs better
17:28fzzzyI'm surprised they stack like that though, I'm pretty sure we wait for the promise, I'm double checking
17:30fzzzyhmm d c o a t e s changed the code a lot recently though
17:31jgruenfzzzy is that like a beetlejuice thing?
17:31fzzzyjgruen: hehe yes
17:31fzzzywouldn't want to summon any ancient demons
17:32fzzzyaha, I see the bug. we are not waiting for the install promise any more due to not returning it. whoops
17:33aswanheh. see me previous comment though.
17:34fzzzyaswan: true
17:35fzzzyI guess the fix is install.install().then(resolve, reject) instead of explicitly listening for onInstallEnded?
17:35aswanor just `return install.install();` ?
17:35fzzzyalthough we might not even need to create our own promise, not sure why we don't just return the one from install
17:35fzzzyheh yeah
17:36aswanyou're also not listening for onInstallCancelled which is raised if the user refuses the permissions
17:36aswanbut amo has the exact same bug :-)
17:36* fzzzy nods
17:37fzzzyI also had to add the onInstallFailed listener to fix a bug
17:37fzzzyI assume the install promise just takes all those events into account?
17:37aswanit is supposed to
17:38fzzzyfix a bug and make the code shorter at the same time. the best kind of patch.
17:39fzzzyi don't think there is anything that can be done about the download not starting right away though
17:40aswanwell if/when we do something about bug 1329884, you could just start them both
17:40firebot NEW, Handle notifications for simultaneous installs better
17:40fzzzywhile I am looking at this code, install.addEventListener('onInstallEnded', () => resolve());
17:40fzzzy could just be install.addEventListener('onInstallEnded', resolve);
17:40fzzzyI have seen that a few times
17:41aswani think the onInstallEnded handler gets called with some arguments, so those will propagate through the Promise if you care about that...
17:41fzzzyok. it can all be replaced by return install.install() though :)
17:43aswanor even `.then(install => install.install())`if you're into code golf
17:44fzzzyhah, right, expression form
17:50b4handfzzzy: Hey man. So I am still stuck with the aboout:neterror. Even tried sauce labs but no luck.
18:01fzzzyb4hand: no worries, thanks for all your work. maybe i'll poke at it soon, or lorchard might
19:28Seburockprice: Thanks for the email. Can I pick this up next week?
19:28ckpriceSeburo: sure
19:30Seburockprice: Thanks. Not off the radar, just getting everything from everywhere right now (+ Egencia fun!).
19:30chuckaswan: gentle nudge at
19:30firebotBug 1310752 NEW, Document mozAddonManager
19:31chuckWould help a great deal :)
19:31aswanchuck: sigh. i know, but such a full plate right now
19:31chuckI know it.
19:31aswannot that that is ever a temporary situation
19:32chuckIn fairness, AMO and us are likely the only-ever consumers.
19:32chuckReally hard to justify it being a priority.
22:14clouserwfzzzy: is your remove-react-router branch working right now? I do a fresh check out, npm install, npm run static, and it fails talking about flow
22:14clouserwsame steps on master work
22:15fzzzyclouserw: it should be. let me check
22:15clouserwerror is "Type imports require valid Flow declaration" in the varianttests.js file
22:23fzzzyclouserw: I see the same. odd
22:27fzzzyeven npm start doesn't work, even though it did before i did gulp distclean
22:36fzzzyclouserw: Fixed. eslint-plugin-flow must have gotten more strict in a newer version? I feel like our package.json should pin all the dependencies to specific versions...
22:56clouserwthanks fzzzy, working for me too
22:56fzzzythanks clouserw
20 Apr 2017
No messages
Last message: 4 days and 7 hours ago