-
Notifications
You must be signed in to change notification settings - Fork 10k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable dumpio when running the tests in order to have some useful debug data #18260
Conversation
It could help to figure out what's wrong in #18259. |
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/34fcd127eefeaf9/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/cd69d4693491265/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/34fcd127eefeaf9/output.txt Total script time: 28.57 mins
Image differences available at: http://54.241.84.105:8877/34fcd127eefeaf9/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/cd69d4693491265/output.txt Total script time: 44.23 mins
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:
What do you think? |
Yes there's some noise :(, especially those exceptions we've because of DecompressionStream (I must file a bug about that). |
Unfortunately I found out that this patch won't help with that particular issue; see #18259 (comment). 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 |
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 |
From: Bot.io (Windows)ReceivedCommand cmd_test from @calixteman received. Current queue size: 2 Live output at: http://54.193.163.58:8877/32bb151d23ef0ea/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @calixteman received. Current queue size: 1 Live output at: http://54.241.84.105:8877/2495d314125b7a2/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/2495d314125b7a2/output.txt Total script time: 29.39 mins
Image differences available at: http://54.241.84.105:8877/2495d314125b7a2/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/32bb151d23ef0ea/output.txt Total script time: 42.76 mins
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. |
No description provided.