mineflayer icon indicating copy to clipboard operation
mineflayer copied to clipboard

Optimize tests

Open wvffle opened this issue 4 years ago • 5 comments

Right now the tests are very slow. It's due to running all of the bots synchronously. Which is painful when testing locally. image image

We could run all of the bots at once and wait for every bot to finish it's task or timeout. I'm speaking of something like await Promises.all(tests)

I don't know if mocha lets us do this but I'm pretty sure that there is some library that can handle that.

wvffle avatar May 11 '20 16:05 wvffle

circle-ci already add parallelism to match the number of available cores, so increasing parallelism is not necessary. It does so by parallelizing over the different versions. Parallelizing within a single version would require to launch more servers so we are sure the bots don't interfere which each others, resulting in more resource usage which might slow down everything. When testing locally, you can launch one test at a time (or a bunch of them using the correct grep command) eg: node_modules/.bin/mocha --grep "1.14.*" --exit this is usually very fast, depending on the test.

Karang avatar May 11 '20 16:05 Karang

Indeed starting only the test you need locally is the best option. I don't think we can afford to start more than one server at a time per circle ci container, it will not get faster. The vanilla server is pretty resource intensive.

Circle ci tests used to take 5min. Do they take more now ?

If latest versions are slower than oldest one, we could put more old version in the first containers of circle ci and less (1) recent versions in the last containers. Is it really necessary though ?

On Mon, May 11, 2020, 18:39 Karang [email protected] wrote:

circle-ci already add parallelism to match the number of available cores. It does so by parallelizing over the different versions. Parallelizing within a single version would require to launch more servers so we are sure the bots don't interfere which each others, resulting in more resource usage which might slow down everything. When testing locally, you can launch one test at a time (or a bunch of them using the correct grep command) eg: node_modules/.bin/mocha --grep "1.14.*" --exit this is usually very fast, depending on the test.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/PrismarineJS/mineflayer/issues/990#issuecomment-626815442, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAR437VVPSPKIMX6B2ZLZFDRRAS3JANCNFSM4M6BOJBA .

rom1504 avatar May 11 '20 17:05 rom1504

circle-ci already add parallelism

That is why I was said is painful when testing locally

Parallelizing within a single version would require to launch more servers so we are sure the bots don't interfere which each other

I don't think that is true. We can simply move the bots to different chunks of map.

resulting in more resource usage which might slow down everything

We can limit number of bots running at once. Also we can make things like chunks be shared between bot instances if we want to limit the resources.

When testing locally, you can launch one test at a time

I know that and I created a PR just after this issue https://github.com/PrismarineJS/mineflayer/pull/991

wvffle avatar May 11 '20 17:05 wvffle

Yeah but what you're proposing would increase test interdependency, ie one test could potentially affect the results of the other ones. And it would also increase complexity of beforeEach afterEach. Is it really needed ? Are tests on circle ci too slow for your use ?

On Mon, May 11, 2020, 19:28 Kasper Seweryn [email protected] wrote:

circle-ci already add parallelism

That is why I was said is painful when testing locally

Parallelizing within a single version would require to launch more servers so we are sure the bots don't interfere which each other

I don't think that is true. We can simply move the bots to different chunks of map.

resulting in more resource usage which might slow down everything

We can limit number of bots running at once. Also we can make things like chunks be shared between bot instances if we want to limit the resources.

When testing locally, you can launch one test at a time

I know that and I created a PR just after this issue #991 https://github.com/PrismarineJS/mineflayer/pull/991

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/PrismarineJS/mineflayer/issues/990#issuecomment-626842141, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAR437SXZJESXKEYPQDOCHLRRAYUDANCNFSM4M6BOJBA .

rom1504 avatar May 11 '20 18:05 rom1504

Speed of circle-ci is not bothering me. It can run in the background for pretty much every commit so it doesn't matter to me.

Testing stuff locally is another topic. Sometimes it's better to test things out locally than on a CI to check if the code works. One can use grep for testing only desired test but sometimes, when editing for example index.js, any test could fail. And instead of pushing 4 commits that try to fix the problem and check CI every time it's better to run all tests locally. Which takes a lot of time.

It's just an idea how to improve the speed of tests.

wvffle avatar May 12 '20 01:05 wvffle