| 03:38 | <TimothyGu> | Domenic: Chrome doesn't implement the fully active part of "check if we can run script", right? |
| 03:42 | <Domenic> | TimothyGu: I couldn't say off the top of my head... I'd find that a bit surprising, as avoiding non-fully active document scripts is something I'd think people like Hiroshige and Kouhei would be excited about. If you have a repro case it'd be worth filing and CCing me+them. |
| 03:43 | <TimothyGu> | Oh okay. don't have a repro or anything but that seemed to be what I remembered |
| 15:10 | <smaug____> | Hmm, why would html:script and svg:script behave differently https://github.com/web-platform-tests/wpt/blob/master/content-security-policy/nonce-hiding/script-nonces-hidden-meta.sub.html#L49 https://github.com/web-platform-tests/wpt/blob/master/content-security-policy/nonce-hiding/svgscript-nonces-hidden-meta.sub.html#L52 |
| 15:12 | <annevk> | smaug____: I think that's a bug in the test |
| 15:12 | <annevk> | smaug____: probably because Chrome has a bug as I discovered elsewhere and they just aligned the test with their impl... |
| 15:12 | <smaug____> | ah |
| 15:13 | <annevk> | smaug____: sigh |
| 15:13 | <annevk> | smaug____: I can prepare another patch |
| 15:13 | <smaug____> | thanks |
| 15:13 | <annevk> | (this is why parametrized tests are nice btw, as in that case this kind of nonsense would be much more easily spotted) |
| 15:13 | <annevk> | Domenic: ^^ |
| 15:14 | <smaug____> | looks like mozilla repo has still the tentative variants of the tests |
| 15:14 | <annevk> | smaug____: yeah I saw that :/ |
| 15:14 | <annevk> | smaug____: ckerschb could pull in the updated tests if he wants, I think that's okay to do |
| 15:14 | <smaug____> | yup |
| 15:14 | <smaug____> | or wait the next merge |
| 15:25 | <annevk> | smaug____: that mistake was present in both SVG files btw |
| 15:25 | <smaug____> | ah |
| 15:26 | <annevk> | https://github.com/web-platform-tests/wpt/pull/21934 |
| 15:26 | <annevk> | smaug____: ^ |
| 15:26 | <smaug____> | yup, looking |
| 15:27 | <smaug____> | annevk: what is the change to script-nonces-hidden.html ? |
| 15:30 | <annevk> | smaug____: it moves "assert_equals(eventList.length, 3);" down, but I also added messages to the other asserts to make them more useful and the diff is the annoying result of those rather basic changes |
| 15:30 | <smaug____> | so just cleanups |
| 15:31 | <annevk> | smaug____: aye |
| 15:31 | <smaug____> | this small nonce thing has required surprisingly lots of time :/ |
| 15:33 | <annevk> | these are the risks with one person doing tests/spec/impl |
| 15:33 | <annevk> | review can only balance so much of that and I don't think we did as careful test review back then |
| 15:49 | <mkwst> | Sorry. :( |
| 15:53 | <jgraham> | mkwst: Since you're here :) referrer-policy/ takes like 10% of the total runtime of wpt (actually more like 13% of the time running tests). Which is the single biggest directory other than css/ (and maybe html/). Do we know if there's any efficiency wins to be had there? |
| 15:54 | <mkwst> | There 100% are efficiency wins to be had. |
| 15:54 | <mkwst> | I think horo@(?) was poking at splitting up the `referer` header length tests, which were likely a big perf regression. |
| 15:54 | <mkwst> | We'd added them to _all_ the tests, but that probably isn't actually helpful. |
| 15:55 | <mkwst> | So, that's a thing we could change. |
| 15:55 | <mkwst> | I haven't looked at the rest in detail in a long time, but there are a lot of tests there that rely on fetches and redirects and navigations, all of which is slow. |
| 15:55 | <mkwst> | We're going to make it worse in a bit by generating tests that do similar work for fetch metadata. |
| 15:56 | <mkwst> | There's likely room for us to collaborate on figuring out a way to test multiple things at once about a given response so that we make a given request once and test all the features, rather than making the request once per feature. |
| 15:56 | <mkwst> | But that's something of a large project. |
| 15:58 | <jgraham> | OK, thanks. Sounds like there is work that can be done, but not a lot of known low-hanging-fruit |
| 16:18 | <annevk> | Wow, referrer-policy is definitely on the extreme side of templating in tests |
| 16:31 | <jgraham> | https://hoppipolla.co.uk/410/wpt-flame.svg is the graph I'm using for the 10% number btw; shows which tests are slow in gecko |
| 16:43 | <annevk> | Encodings also seems excessive |
| 16:43 | <annevk> | Or maybe lots of things are tested poorly |
| 16:45 | <gsnedders> | I presume encoding is caused by encoding/legacy-mb-* |
| 16:45 | <gsnedders> | yeah, it is |
| 16:47 | <jgraham> | Looks like we spend ~10 minutes on each of those |
| 17:27 | <annevk> | Unfortunately the way those tests are written also makes it hard to refactor them |
| 17:28 | <annevk> | jgraham: gsnedders: starting with some kind of coding guidelines around tests might be good |
| 17:28 | <annevk> | jgraham: gsnedders: might have kept the free-for-all thing going for a bit too long? |
| 17:36 | <gsnedders> | annevk: as long as we have two-way syncing, it's hard to get buy in for anything stricter than existing browser testsuites :( |
| 17:37 | <gsnedders> | annevk: also unclear how to enforce any sort of coding guidelines with so many potential reviewers (i.e., basically everyone with commit access to browsers) |
| 17:43 | <annevk> | gsnedders: the status quo where it depends on who you ask for review is also rather terrible, so I'd like us to at least try to do better |
| 17:44 | <annevk> | gsnedders: perhaps it doesn't prevent bad stuff from getting in, but at least there's some kind of baseline to measure things against |
| 20:19 | <Krinkle> | jgraham: nice use of flame graphs :) - how was that generated? |
| 20:19 | <Krinkle> | instrumented within the JS codee? |
| 20:34 | <gsnedders> | Krinkle: pretty sure it's post-processing of the log files from the runner |
| 20:35 | <gsnedders> | Krinkle: (where the runner is Python) |
