mozilla :: #ateam

6 Jan 2017
11:59pyangato: ping
12:38jmaher|afkpyang: I hear you are interested in test selection :)
12:39atopyang: You sent me a content-less ping. It would be faster if you just asked your question (-:
12:41whimbooato: i wonder if we call close() for a window, if we should set to null. Or would that violate something
12:41whimbooright now it persists and points to a dead element
12:42atowhimboo: Its a good question. Unfortunately curBrowser and browser.Context is magic and I dont know the answer.
12:45whimbooato: found a couple of issues in close() btw.
12:45whimbooand I'm going to make it all promise based
13:06pyangato, hey I just want to know if execute_script takes dict as recommended option
13:06atopyang: For script_args you mean? It takes a tuple or a list.
13:07pyangato, yes, but it supports dict as well
13:07atoIts not meant to do that.
13:08atoscript_args is what ends up as `arguments` in the injected JS function.
13:09pyangya, I learn from example which took from __webDriverArguments
13:10atoscript_args gets applied to the function, so {foo: 42} will end up as [].
13:10atoBut I do see that Marionette does not do a type check on the input type.
13:12atoI filed about adding type checks.
13:12bugbotBug 1329184: Marionette, normal, nobody, NEW , Missing type check for Execute (Async) Script in Marionette
13:15pyanghmm.. that leads to use tuple or list.
13:16pyangato: thanks I think I got the point
13:17atopyang: If you want to pass in a dict, you need to pass it as the first argument to the tuple.
13:17atopyang: E.g. script_args=({"foo": 42},)
13:17pyangya I see like you filed in
13:17bugbotBug 1286312: Mochitest, normal, pyang, ASSIGNED , Add mochitest option to run tests using https
13:18atopyang: Then in the script you can go: let a = arguments[0], and will be 42.
13:32AutomatedTesterato: hey, have you tried the TLS stuff with geckodriver?
13:50whimbooato, AutomatedTester: where can I find details about this issue 7?
13:51whimboothere is nothing under
13:51AutomatedTesterwhimboo: the issues are auto generated, it means we need to write prose to fill it in
13:52whimboobut there is no real issue for tracking this?
13:52AutomatedTesterthat's what I said :)
13:52whimbooi ask because when I click to file a new issue I get forwarded to
13:53whimboowhere i can file a new one
13:53AutomatedTesterwhimboo: thats where spec issues are tracked
13:54whimbooso in the above case I don't understand what's needed
13:54whimboo"This can prompt unload, in which case the operation really failed."
13:54whimboothis is true if the last tab of a window gets closed
13:54whimbooso you see a unload event of the chrome window
13:55whimboowhat i'm expected to do with it?
13:55AutomatedTesterI think its meaning unload events from content e.g. "Are you sure you want to leave this page?"
13:56whimboonow it makes more sense :)
13:56AutomatedTesterin that case we need to throw "Unexpected alert open" error and finish the command
13:56AutomatedTesterfinish the command as in return not do the steps after that
13:57whimbooAutomatedTester: or like for other commands: Handle any user prompts and return its value if it is an error.
13:57whimboowhereby this alert is triggered after the call to close tab
13:57whimboobut might still be an option
13:58whimbook, something for later
14:07atoAutomatedTester: It appears geckodriver isnt passing capabilities on to Marionette, and that webdriver-rust is missing the insecure certificate error type.
14:07atoAutomatedTester: Fixing this now.
14:07AutomatedTesterato: ok cool, I traced the issue to inside marionette
14:08atoWhat did you find?
14:08AutomatedTesterwhen we are doing the sessionCapabilities its not working propery
14:08whimbooato: so for close() the spec says that we have to return the remaining window handles. how can i easily call the getWindowHandles command and return its result?
14:08atoAutomatedTester: I dont think this is in Marionette, but geckodriver isnt passing the capabilities on properly.
14:09atowhimboo: You want to abstract the implementation of getWindowHandles and call that from getWindowHandles and your new code.
14:10whimbooato: i see. so adding a generic method then
14:10AutomatedTesterato: thats what I found
14:10AutomatedTesterwell... I removed my extra logging lines
14:10whimbooato: do we have a good place for those helpers? or should all be put into driver.js
14:12atoAutomatedTester: Wait, youre right. I added some further logging after we return from setSessionCapabilities, and what happens is that geckodriver is sending the capabilities as mandated in the spec, but Marionette only looks at the top-level dictionary. This will be fixed when lands.
14:12bugbotBug 1326534: Marionette, normal, ato, ASSIGNED , Capabilities parsing that conform to WebDriver
14:12AutomatedTesterato: awesome
14:13whimbooato: so i assume that is the reason why people are having issues with it?
14:13whimboowhen using selenium
14:14atowhimboo: We dont really have a good place to put it. I think Id r+ a patch that just calls GeckoDriver#getWindowHandles(cmd, resp) for now.
14:14atoYes, I think so.
14:15atoMore WPT tests would be good.
14:15whimbooato: i tried that but i would have to wait for the promise to fulfill. I thought it might be better let the dispatcher do it
14:18atoAutomatedTester: r?
14:19AutomatedTesterato: I will merge once travis is finished
14:21atoAutomatedTester: Good thing you pointed this out. I think we might want to do another release.
14:24atojgraham: Will you do a release of rust-mozrunner so we can get it in?
14:26atoAutomatedTester: The insecure certificate error seems to be missing in Selenium too.
14:26atoAutomatedTester: I guess maybe it will trickle down to some form of WebDriverException so that it isnt fatal?
14:26AutomatedTesterato: I was looking to add it to Selenium which is how I came across this
14:33atomaja_zf|afk, whimboo: Do you want to have another look at
14:34atomaja_zf|afk, whimboo, AutomatedTester: The test failure was caused by a typo, so fixed that. Also made functional matching tests more granular.
14:40whimbooato: sure. would you have a minute to check my smallish review? its just a rubberstamp because i moved the changes out to a new commit
14:48atowhimboo: Im getting there (-:
14:48atoHonestly havent done anything else than reviews, needinfos, and fixups all day.
14:48whimbooato: just another small question. what is the up2date way to return a value from a command? resp.body, just return, a promise?
14:49atoYou can do all of those.
14:49atoYou can yield the promise, assign to resp.body, return an object, or return a promise.
14:52whimbooso i will return the promise with the window handles
14:52whimbooas value
14:59jgrahamdavehunt: Published a new mozrunner_rust, but you'll need to build geckodriver yourself to get the changes
14:59jgrahamMight do a minor version release soon with that and some other fixes
15:00AutomatedTesterjgraham: we need to do a webdriver-rust release too
15:00AutomatedTesterand then prolly a geckodriver one
15:03jgrahamRight, that's the "some other fixes"
15:05AutomatedTesterok cool :)
15:20davehuntjgraham: I can wait, thanks though! :)
15:33jgrahamahal: You back around today?
15:33ahaljgraham: yeah
15:34jgrahamahal: Mind if I release mozprocess for the benefits of servo? Or do you want to do it? (I guess one person will have to do review and the other make the version number change :)
15:36ahaljgraham: sure, go ahead, no concerns from me
15:36ahalI can review the bump if you like
15:46jgrahamahal: OK, version number bump is in your review queue
16:12whimbooato: nice. close() works as expected now. Next item is chrome windows
16:12atowhimboo: I had a WIP patch for this, but happy to take yours instead since mine is probably bitrotted.
16:13atowhimboo: Note that since Firefox doesnt let you close the last open tab, we need to lie to the client and return an empty array when the number of open windows is 1.
16:13whimbooato: which was not uploaded. so i started from fresh. Lets see how you will like it
16:13whimbooato: why cant we close the last tab?
16:14whimboothat's acgtually a usecase i havent added to the tests
16:14whimboothere is a pref which you can flip
16:14atowhimboo: Closing the last tab will presumably exit Firefox on Linux and Windows.
16:15atowhimboo: The point is that when geckodriver receives an empty array after closing a window, it will know its time to kill the Firefox process.
16:15whimbooato: so right now we close the session
16:15atoYeah, thats wrong.
16:15atoAll my doing (-:
16:15whimbooAutomatedTester said, it might be added to the spec
16:15atoI want the Close Window command to leave the session open.
16:15whimbooato: mind replying to the bug then?
16:15atoWhich bug is this again?
16:16whimboobug 1311350
16:16bugbotBug Marionette, normal, hskupin, ASSIGNED , close() and closeChromeWindow() do not wait until the underlying window has disappeared
16:16atoNote, leave it open _in Marionette_.
16:16whimbook, easy workaround
16:16whimbooto do
16:17atoWe cant close the session in Marionette and shut down the TCP connection because theres no way (with Geckos current socket implementation) to guarantee that the last message will get sent before we shut down Firefox.
16:17atoSo we will have geckodriver inspect the return value and kill Firefox instead.
16:18whimbooalright. so i will get this in. thanks
16:19atoI will clarify in a comment.
16:36atoTIL Mozilla has a mailing list alias for those who dont have Apple gear checked out.
16:42jmaherodd, that sort of implies that the majority are apple users
16:48RyanVMsuddenly comes to mind
17:07atojmaher: My subjective impression is that thats true for San Francisco, Mountain View, and Toronto.
17:09jmaherI recall asking specifically for a non mac machine about 6 years ago as a replacement and what did I get in the mail....a macbook pro
17:09jmaherpossibly it depends on what IT is handing out
17:09atoIn the London office today we seem to be 50/50 on Mac and Linux (not all developers).
17:10jmaherin the Cary, NC office we are 100% not on Mac, a healthy mix of windows, linux, and android
17:10atoWhat you work on decides what tool you use to a wide degree I guess (-:
17:11jgrahamWell not really. Most of use could use macOS or linux or even Windows per personal preference
17:11atoDepending on how much pain you wish to inflict upon yourself (-:
17:11jgrahamI think that's true of a lot of people who aren't literally doing Fx frontend for a specific platform
17:12atoIf I wasnt working on native software development (or software engineering, really) I might not use my current configuration.
17:12atoIts hard once you get used to something, though.
17:12atoChange I mean.
17:13jmaherwell, I couldn't even build gecko for years on osx, which is why when I had a macbook pro, I ran linux native on there
17:13atoThey make nice laptops (-:
17:16jmahermy mother-in-law got an iphone7 recently and she wanted some photos on there and there was no way to copy files to the phone like you would on a windows phone or android phone, you were forced to use apple itunes (that sounds like a music player, not a file copy tool for photos) or some nasty hacky software
17:16tedwlach|lunch: when you're done with lunch, perfherder question: is there a way to get a pushlog link for the set of data points currently visible on the graph?
17:16tedi'm looking at a graph that has a dropoff and i'd like to try to figure out what changed there
17:16jmaherted: can you link the graph?
17:17tedit's a little confusing, because we also apparently changed instance types there
17:17tedbut it does look like the timings on the previous instance types dropped, or at least got less noisy
17:17atojmaher: Every time Maja and I try to get some photos off her iPhone, all the Apple services let us down (photo syncing, sharing, iCloud) and we end up sending it by email instead.
17:17jmaherso you want to see what commit caused it to be less noisy?
17:17atojmaher: Good, old email. (-:
17:18jmaherato: lol, that is what I ended up using after an hour of fiddling around
17:20jmaherted: you can click on any data point and see the revision (link to hg), the job (link to treeherder), or compare (link to perfherder compare to another revision)
17:21tedjmaher: yeah, but i don't know which data point to use
17:21jmaherted: sometimes I click on a job link to treeherder and then click the button for more data at the bottom of treeherder to see a wider range, you could even use fromchange/tochange in urlparams
17:21tedi picked a few at the edge of the cliff randomly and nothing looked obvious
17:21tedjmaher: guess i'll file a bug!
17:21tedshould be easy enough to generate a pushlog link for the whole visible range
17:21jgrahammaja_zf: Thanks! Grapheme clusters are intentionally unsupported at the moment, since I didn't spend time figuring out if there is a library that will tell us if a string is a single grapheme cluster or not
17:22maja_zfsounds good
17:22jgrahamAutomatedTester, ato: Anything unlanded you were hoping to get into a point release of geckodriver?
17:23atojgraham: I think a release that introduces new behaviour (insecure certificate error) should be a minor release, e.g. 0.13.0.
17:23atojgraham: And yes, the rust-mozrunner change davehunt made.
17:24jgrahamato: We will get that for free
17:25atojgraham: Yes, did you release a version and bump the version in geckodrivers Cargo.toml?
17:25jgrahamato: That's a major version bump of webdriver, but not of geckodriver imo
17:25atojgraham: This goes without saying, but we also need to change the as_u64 to as_i64 when ugprading webdriver-rust.
17:26tedjmaher: FWIW
17:26bugbotBug 1329240: Perfherder, normal, nobody, NEW , Add a link to the pushlog for the visible changeset range
17:31jmaherted: cool
17:36tedjmaher: another perfherder-related question
17:37tedis there any UI to go from a try push to a compare-talos view automatically?
17:37ted"give me a compare view for this push vs. its base revision"
17:37jgrahammaja_zf: So I randomly noticed whilst updating that we already transitively depend on something called unicode-segmentation which offers the functionality we want. Not going in this release, but I guess it won't be hard to add in the future
17:37jmaherin the 'performance' section of the information popup that is displayed when you click on a job in treeherder
17:37jmaherted: "compare to another revision"
17:38tedjmaher: ahh, build jobs don't get those
17:38tedguess we could fix that
17:38maja_zfjgraham: yay!
17:38jmaherted: taskcluster builds get that info
17:39tedoh huh, so they do
17:39tedthanks, that works then
17:39RyanVMgbrown: I see you marked a dep against bug 1245921
17:39bugbotBug Developer Tools, enhancement, gtatum, RESOLVED FIXED, Migrate toolbox tabs to HTML by using a react component.
17:39RyanVMI think that should be backed out
17:39tedjmaher: thanks!
17:40jmaherit should auto populate the previous revision for you ted; if not there is a link to use base revision
17:40RyanVMit's made some chunks of linux32 debug devtools permafail
17:40tedjmaher: yeah, that works well, very handy
17:40* ato sets the BMO UI option to the old one
17:40jmaherted: if you ever see parkouss online, he made that work
17:40jgrahamHmm, if the new UI is the one I've been using for the last couple of years, it's better
17:45tedjmaher: i met him at an all hands, didn't i?
17:46jmaherted: yeah, he came to whistler and orlando
17:48jmahergbrown: your wpt changes on inbound are sticking
17:48gbrownyes, so far, so good
17:49jgrahamato: OK, just waiting on a changelog then
17:49atoEh, I thought I pasted a link.
17:49gbrownyou know, I'm still waiting for a loaner. loaners seem very slow lately.
17:50jmahergbrown: yeah, it is actually not a good workflow these one click loaners, as you have to be around for the next hour available to pick it up when it is ready or lose it
17:50wlachfirst you tell me I need five clicks, then you tell me I need to wait an hour?
17:54jmaherwlach: then I say...screw it, let me just push to try and get results faster
17:54wlachI guess at least they weren't called "instant 5 click loaners"
17:55atojgraham: Oh, if you havent pushed the tag yet, make sure its an annotated tag.
17:56jgrahamOh, I just did
17:56atoOK, its not a big one.
18:04davehuntAutomatedTester: did you see
18:06malayaleecoderjmaher: just curious, where do you report an error in MozillaWiki?
18:07malayaleecoderjmaher: btw, isn't QoC happening for this Quarter?
18:16jgrahamdavehunt, whimboo|bbl: I didn't know you were doing a talk at fosdem. Should be good! (not that I will be there or anything)
18:16whimboo|bbljgraham: we hope so! :)
18:16davehuntjgraham: I haven't been to FOSDEM in 4 years, which is also when I last did a joint talk with whimboo. I'm looking forward to it.
18:18atoI used to go to FOSDEM every year, but decided my last time was when it was freezing and snowing. Also I crashed a car driving there.
18:22wlachrwood: would you be interested in working on bug 1329224?
18:22bugbotBug Perfherder, normal, nobody, NEW , Add a "share" link to get a shortened URL for graphs etc
18:22wlachrwood: at least the part that I mentioned (shortening the existing links)
18:23rwoodwlach: sure, looks interesting
18:24wlachted: would you be ok if reduced the scope of the bug to just shortening the links at first?
18:26tedwlach: seems fine, it's not a super critical bug, just would be nice to have shorter URLs
18:26rwoodok cool
18:26wlachted: yeah it's bitten me more than a few times
19:09jmahermalayaleecoder: hey, done with my meeting- I am not sure where to report an error in mozillawiki
19:10jmahermalayaleecoder: I assume bugzilla, a handful of wiki related components, including:
19:14haikthis the right place to ask treeherder user questions? Such as, is there a way to determine which test group a test will run in? i.e., which bcX will my new browser chrome test run in?
19:17jmaherhaik: I thought there was a mach tool for that, normally I just examine each of the logs
19:20ahaljmaher: I don't think that tool really works anymore
19:20ahalwe should probably remove it :/
19:20ahaland it only ever worked in narrow circumstances in the first place
19:21jmaherok, lets remove itthen
19:21haikjmaher: ah, that must be find-test-chunk
19:22jmaheryes, that is the magic name
19:22haiksounds like, check the logs is the answer
19:28jmaherahal: lol, we filed dups:
19:28bugbotBug 1329285: mach, normal, nobody, NEW , remove |mach find-test-chunk| as it isn't reliable and can be misleading
19:28ahaljmaher: dang
19:28ahaljmaher: you can choose which one you like better
19:28jmaherone number off
19:29jmaherdup'd mine to your bug
19:29ahaljmaher: thanks.. while we're on the topic of removing things, do you know if --bisect-chunk still works/is useful?
19:30jmaherahal: I know it worked this past summer
19:30jmahernot sure if it is useful though
19:31ahaljmaher: ok, I guess we should just leave it in then
19:31jmaherit isn't really used
19:31ahalthere is a lot of logic associated with that feature
19:32jmaheryeah, it would clean stuff up
19:32* ahal shrugs
19:33ahalshould probably wait for mochitest tests anyway
19:34ahaljust so there's less of a chance of us breaking something obscure while trying to pull it out
19:34jmahergot it
19:34ahalI have a plan to write some python unittests for the mochitest harness sometime this quarter
19:34ahaland hopefully get someone to flush the suite out after a couple are running
19:41tedahal: hooray!
19:42tedalso, in end-to-end build time news, i have a patch to make windows builds 3-5 mins faster
19:42tedso that's nice
19:42ted , although i wound up doing something entirely different than the bug as originally filed
19:42bugbotBug 1323479: Build Config, normal, nobody, NEW , Investigate whether alternate dump_syms is faster on Windows
19:43ahalted: nice! for even more good news, here's a try run with mozbase tests running outside of make check:
19:43tedahal: awesome!
19:43ahalthough it depends on patches I expect will need to go through several iterations of review
20:29jmaherahal: that is great news
20:39rwoodjmaher: thank you for the review (eh)
21:48mcotefyi I'm adding a few relevant newsgroups posts to the list from people other than me
21:48mcotee.g. wlach's post about RFCs
21:48mcotefigure we should draw attention to them in the meeting
21:48mcote*adding to Monday's agenda
7 Jan 2017
No messages
Last message: 224 days and 23 hours ago