Skallwar / suckit

Suck the InTernet
Apache License 2.0
733 stars 38 forks source link

Use local webserver for speed testing #108

Closed CohenArthur closed 3 years ago

CohenArthur commented 3 years ago

I wonder if we should change tests/benchmark.py as well? Since it is a quicker test, I think it matters less.

Skallwar commented 3 years ago

I wonder if we should change tests/benchmark.py as well? Since it is a quicker test, I think it matters less.

I think we should keep it as it is now. I like the idea of real-world performance when comparing to httrack

CohenArthur commented 3 years ago

From what I understand we need to launch local_server_setup.sh by hand then launch speed_regression.py. If this is true, can we try to run local_server_setup.sh from speed_regression.py?

Sure. I wanted to keep it separate so that it's more of a "unix" philosophy. The other issue is that we need to wait for the server to be launched, so it adds a layer of complexity to the script. If we just keep it as this, then two terminals are needed and you can easily see when the webserver is up. We could add something like echo "Web server is up! in green when launching the web server

Skallwar commented 3 years ago

I wanted to keep it separate so that it's more of a "unix" philosophy

My idea is to keep them separated but one is calling the other.

If we just keep it as this, then two terminals are needed

That what bothers me :sweat_smile:

The other issue is that we need to wait for the server to be launched, so it adds a layer of complexity to the script

Waiting for "WEBSERVER UP" on stdout seems not that complicated, am I wrong?

CohenArthur commented 3 years ago

That what bothers me 😅

You can also launch it in the background using ./setup_local_server.sh & if you plan on running tests multiple times. But I would be fine with doing it your way :) I'll tweak it

Waiting for "WEBSERVER UP" on stdout seems not that complicated, am I wrong?

Sure that wouldn't be too complicated haha

Skallwar commented 3 years ago

You can also launch it in the background using ./setup_local_server.sh &

Yeah, it works. But I sure I will forget it's running in the background haha!

But I would be fine with doing it your way :) I'll tweak it

It's up to you. I can live with it if it's correctly explained somewhere

CohenArthur commented 3 years ago

@Skallwar the speed regression script now launches the webserver locally and handles any exception in order to stop it correctly and not have a zombie webserver