w3 :: #testing

21 Apr 2017
00:52rbyersjugglinmike: Thanks for pushing on the ChromeDriver hang. johnchen@ is the one person we have working on chromedriver ATM so I'm happy to see he's tracked down the root cause (GLib bug).
00:52rbyers.. I filed https://bugs.chromium.org/p/chromium/issues/detail?id=713947 to track getting some sort of permanent fix and am pushing for it to have high priority.
00:54rbyers.. I agree with your point that we don't want every user of chrome automation to keep getting burned by this. That said, I see no value in holding WPT infrastructure hostage here - we can always threaten to remove the hack from WPT in a few months if there hasn't been traction on the proper fix...
07:53* davidgreens slaps logbot around a bit with a large fishbot
07:53* davidgreens slaps logbot around a bit with a large fishbot
07:53* davidgreens slaps MikeSmith around a bit with a large fishbot
07:53* davidgreens slaps lmclister______ around a bit with a large fishbot
07:55* davidgreens slaps botie around a bit with a large fishbot
10:07zcorpanhttps://github.com/w3c/web-platform-tests/pull/5402/files/adfd687537ec152da6d7a0e678c4ff28e0456748..afa70a950ff98bf7ff419482808cdfedd6094c12#r112565245 ... any opinions on this? personally i don't mind tests relying on ASI, but i try to avoid it myself and i wouldn't mind a lint check for it (not sure it's possible/practical to lint though, especially with wpt-serve substitutions)
10:09jgrahamzcorpan: I guess "make bz happy" is a good mantra to live by :)
10:09zcorpancertainly :-)
10:09jgrahamLiek you say linting might be non-trivial; there was talk of using eslint but afaik nothing has actually come of it
10:10annevkI'm not going to address those comments without having a styleguide enforced on everyone though
10:10annevkGetting different feedback depending on the reviewer is frustrating and not worth it
10:11annevkI don't think I ever consciously type a space after "if" for instance
10:11zcorpani think so far we have avoided a style guide for similar reasons we avoid required metadata. i.e. it's more valuable to get tests actually submitted
10:15jgrahamI think that "space after if" isn't worth changing, but "ASI makes the code hard to read" seems like a reasonable concern
10:18annevkWell, put it in a style guide; I personally find all the semicolons rather distracting
10:20annevkjgraham: I thought we had stopped Chrome from blocking a PR?
10:20annevkjgraham: https://travis-ci.org/w3c/web-platform-tests/builds/224287583
10:42jgrahamannevk: You need to rebase onto master
10:42annevkjgraham: I already did?
10:43annevkOh I guess I failed
10:43annevksigh
10:45annevkHmm, someone forgot to use squash on one of my PRs and now "LF it" is a commit on master
10:46annevkT?IF
10:48annevkjgraham: btw, the biggest problem with bots spam is that I can never quite tell if it's just bots or if a human also left a comment maybe, so you always feel forced to check
10:53jgrahamannevk: I agree that bot spam is an issue. But the reality of the situation right now is that it's non-trivial to fix the problem in a good way and we are resource constrained. I am supposed to be working on making css tests work in Mozilla Central. Bob Holt is on vacation. MikeSmith is in China. jugglinmike has been working on the Chrome stability issue and is writing tests. I do understand the issue (I am also a major contributor to this repo after a[CUT]
10:53jgraham... to characterise it as "drop everything" severity.
10:54jgrahamFixing it in the right way probably means merging bots so they can all edit the same comment without code duplication
10:54jgrahamThat isn't an afternoon fix.
10:54annevk"after a[CUT]"?
10:57annevkI understand the resource constraints, but there's also been some very recent changes that added a bunch of bots. Maybe that all happened due to the submodule merges and nobody foresaw the problem though
10:57annevkBut yesterday we were still debating whether it's a problem worthy of solving
10:57annevkAt least, that's the impression I got from the pushback
11:09jgraham... after all), but it's hard ...
11:10jgrahamannevk: Sorry in that case I was unclear. I have always agreed that reducing the bot spam is an issue worth addressing. I only disagree that disabling reviewable so that people who aren't using it don't have to look at the link is a good tradeoff given how useful the link/integration is for people who do use it.
11:11jgrahamAnd yes, the submodule merges made this worse; I have tried to address some of the issues there (e.g. I have a PR to hopefully reduce the spam from the coverage bot)
11:12jgrahamMaking the lint only comment if there's an error might also be low hangling fruit, but that requires code invesigation
11:13jgrahamI don't know if it has access to that information at present
11:13jgrahamContributions here are very welcome.
11:17annevkjgraham: I could look at the lint code if I knew where to start, but probably not this week
16:00jgrahamMikeSmith, jugglinmike: I don't suppose either of you has an entivonment set up for testing the prbuildbot script do you? I made some change to try and combine comments, but actually setting it up looks exhausting.
16:00jgrahamAlso I don't think this simplistic approach is going to work very well
16:01jgrahamBecause we hit the comment size limit fairly often with uncombined comments.
16:03jgrahamSo a reasonable alternative interpretation of the above is that I just wasted four hours proving something that I initially claimed was non-trivial is actually non-trivial rather than doing something I'm supposed to be working on
16:03jgrahamSo yeah
16:10jgrahamjugglinmike: ^ made the changes you requested. Well the ones I think are useful in the short term anyway.
16:46jugglinmikethanks jgraham. I just replied; I hope my suggestion seems fair
16:49jgrahamjugglinmike: Perfectly reasonable
16:51jugglinmikeahh
16:53jgrahamjugglinmike: How's that?
16:56jugglinmikeGreat!
16:56jugglinmikeOne process-related thing. I'll note it on the issue itself
17:38jgrahamjugglinmike: Updated the commit message; I'll merge now
17:39jugglinmikeGreat!
17:41jgrahamI started looking at the testharness.js testsuite but didn't get too far
17:41jgrahamIn general it looks good though
17:45jugglinmikegood, good
17:46jugglinmikeinteresting that | takes precedence over ` in GitHub flavored Markdown tables
17:46jgrahamI wonder if there's a way for someone to throw money at travis so we get faster builds
17:47jugglinmiketoo bad there's no standard here
17:57gsneddersjgraham: http://wptserve.readthedocs.io/en/latest/
17:57gsneddersjgraham: that's b0rked
17:58jgrahamgsnedders: Ah
17:58gsneddersjgraham: cool URIs don't change, damnit
17:58jgrahamBut real URIs are a) called URLs and b) change
17:59jgrahamThe world sucks, go listen to emo or something i dunno
17:59* jgraham going home now
18:00jgrahamgsnedders: Remind me to change travis to pass before the CSS build is done
18:00jgraham(alternatively: please change travis to etc.)
18:00jgrahamOur jobs are pretty slow :/
18:00gsneddersjgraham: am not working today ;P
18:01gsneddersjgraham: next time i'm doing work will be when I crash your office :P
18:03jugglinmikehave a good weekend, jgraham
19:10jugglinmikejgraham: Are there any issues with making testharness.js dependent on Promise?
19:10jugglinmikei.e. even for non-Promise tests
19:33jgrahamjugglinmike: YEah, maybe for servo
19:33jugglinmikewell darn it
19:35jugglinmikejgraham: I should have expected this, but making `add_cleanup` recognize Promise return values entails making a good deal of the internals asynchronous
19:45jugglinmikejgraham: This is kind of throwing a wrench in my plans. Not sure if it means the feature is doomed, or if I should be re-writing to use semaphores and callback functions, or maybe something else
19:45jugglinmikeDo you have any advice?
19:49jgrahamjugglinmike: I would need to think about it and look at the code
19:49jugglinmikeOkat
19:49jugglinmikeOkay*
19:49jugglinmikeI'll see if I can round it out using Promises
19:49jugglinmikefor now
19:54jugglinmikejgraham: Another question
19:54jugglinmikeCurrent behavior seems a little wrong to me: http://w3c-test.org/resources/examples/apisample18.html
19:55jugglinmikeIn that it only reports a single sub-test: the parent test
19:56jugglinmikebut the iframe defines an additional test that isn't run
19:56jugglinmikeSo that ought to be picked up by `fetch_tests_from_window`
20:36jgrahamjugglinmike: Agreed
20:37jugglinmikecool, cool
20:37jugglinmikeI'm finding other bugs this way, too
22 Apr 2017
No messages
   
Last message: 5 days and 18 hours ago