Closed calixteman closed 2 weeks ago
/botio test
Command cmd_test from @calixteman received. Current queue size: 0
Live output at: http://54.241.84.105:8877/34fcd127eefeaf9/output.txt
Command cmd_test from @calixteman received. Current queue size: 0
Live output at: http://54.193.163.58:8877/cd69d4693491265/output.txt
Full output at http://54.241.84.105:8877/34fcd127eefeaf9/output.txt
Total script time: 28.57 mins
different ref/snapshot: 12
different first/second rendering: 2
Image differences available at: http://54.241.84.105:8877/34fcd127eefeaf9/reftest-analyzer.html#web=eq.log
Full output at http://54.193.163.58:8877/cd69d4693491265/output.txt
Total script time: 44.23 mins
different ref/snapshot: 5
Image differences available at: http://54.193.163.58:8877/cd69d4693491265/reftest-analyzer.html#web=eq.log
Hm, I'm a bit on the fence about this. On the one hand it did already provide useful information and surfaced issues such as #18246 and the logs below, but on the other hand it also generates a lot of log spam for especially the integration tests which makes navigating the logs harder.
I'd feel better about this if we could tidy the logs up a bit first:
JavaScript error: http://127.0.0.1:62573/build/generic/web/viewer.mjs, line 3023: Error: Not implemented: #createBundleFallback
already don't look particularly good (in that sense it proves the value of this setting though).dumpio
only for the reference tests and not for the unit/integration tests, at least for the time being until we figured out what's going on for #18259 (and maybe after that disable it again).What do you think?
Yes there's some noise :(, especially those exceptions we've because of DecompressionStream (I must file a bug about that). We should try to fix the createBundleFallback one. That said in considering that it helped to fix one bug (in integration tests) and it's the best idea I've for the moment to be able to potentially catch something from this intermittent failure, I'd say we should do it.
It could help to figure out what's wrong in https://github.com/mozilla/pdf.js/issues/18259.
Unfortunately I found out that this patch won't help with that particular issue; see https://github.com/mozilla/pdf.js/issues/18259#issuecomment-2173874135.
I'm still somewhat in favor of doing this, but ideally once the integration test logs are cleaned up at least a little bit, to avoid the logs becoming unwieldy to read through. I'll create dedicated issues for the most important/spammy ones because this patch did nicely surface those issues, so that in itself is really good.
I have created the two issues above for the most important log spam problems here. I think once those are resolved we should be in a state where we can enable dumpio: true
, which coming to think of it could also help with future patches because it'll surface new logs for PRs instead of them going by unnoticed as was the case until now.
Can we perhaps revisit this one now, given the PRs that landed earlier today?
Yes, overall the logs should be in a much better state now that the two mentioned issues have been fixed. @calixteman Could you rebase this patch and trigger a new test run to verify that the logs should be in a good enough state now that we can enable this?
/botio test
Command cmd_test from @calixteman received. Current queue size: 2
Live output at: http://54.193.163.58:8877/32bb151d23ef0ea/output.txt
Command cmd_test from @calixteman received. Current queue size: 1
Live output at: http://54.241.84.105:8877/2495d314125b7a2/output.txt
Full output at http://54.241.84.105:8877/2495d314125b7a2/output.txt
Total script time: 29.39 mins
different ref/snapshot: 19
different first/second rendering: 1
Image differences available at: http://54.241.84.105:8877/2495d314125b7a2/reftest-analyzer.html#web=eq.log
Full output at http://54.193.163.58:8877/32bb151d23ef0ea/output.txt
Total script time: 42.76 mins
different ref/snapshot: 6
Image differences available at: http://54.193.163.58:8877/32bb151d23ef0ea/reftest-analyzer.html#web=eq.log
This has clearly proven its value, and the logs are much more under control now. Good idea to enable this! I'll create a meta-issue to clean up the logs further, but it's not blocking landing this anymore.
It could help to figure out what's wrong in #18259.