-
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
Remove the streamqueue
dependency
#18483
Conversation
The `streamqueue` dependency is only used for the test targets in the Gulpfile to make sure that the test types are run in series. This is done by modelling the test processes as readable streams and then having `streamqueue` combine them into a single readable stream for Gulp that processes the inner readable streams in series (in contrast to the `ordered-read-streams` dependency which is very similar but processes the inner streams in parallel). However, modelling the test processes as readable streams is a bit odd because we're not actually streaming any data as one might expect. Instead, we only use them to signal test process completion/abortion. Fortunately nowadays, with modern Gulp versions, we don't need readable streams and `streamqueue` anymore because we can achieve the same result with simple asynchronous functions that can be passed to e.g. `gulp.series()` calls. Note that we already do this in various places, and overall it should be a better fit for test process invocations.
5f7c2be
to
1b9981c
Compare
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://54.241.84.105:8877/afcc203273b9d7c/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://54.193.163.58:8877/a331256530fa111/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/afcc203273b9d7c/output.txt Total script time: 27.78 mins
Image differences available at: http://54.241.84.105:8877/afcc203273b9d7c/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/a331256530fa111/output.txt Total script time: 44.21 mins
Image differences available at: http://54.193.163.58:8877/a331256530fa111/reftest-analyzer.html#web=eq.log |
/botio-linux browsertest |
From: Bot.io (Linux m4)ReceivedCommand cmd_browsertest from @timvandermeij received. Current queue size: 0 Live output at: http://54.241.84.105:8877/df096da383831f7/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/df096da383831f7/output.txt Total script time: 20.24 mins
Image differences available at: http://54.241.84.105:8877/df096da383831f7/reftest-analyzer.html#web=eq.log |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing a dependency is even better than replacing it with another one :-)
r=me, thank you!
The
streamqueue
dependency is only used for the test targets in the Gulpfile to make sure that the test types are run in series. This is done by modelling the test processes as readable streams and then havingstreamqueue
combine them into a single readable stream for Gulp that processes the inner readable streams in series (in contrast to theordered-read-streams
dependency which is very similar but processes the inner streams in parallel).However, modelling the test processes as readable streams is a bit odd because we're not actually streaming any data as one might expect. Instead, we only use them to signal test process completion/abortion.
Fortunately nowadays, with modern Gulp versions, we don't need readable streams and
streamqueue
anymore because we can achieve the same result with simple asynchronous functions that can be passed to e.g.gulp.series()
calls. Note that we already do this in various places, and overall it should be a better fit for test process invocations.I have run all affected test targets locally to confirm that they still work and run the test types in series like before. Moreover, I have triggered the error paths to make sure those didn't change either.
Fixes #18474.