locustio / locust

Write scalable load tests in plain Python 🚗💨
https://locust.cloud
MIT License
25.1k stars 3k forks source link

Report name #2947

Closed obriat closed 2 weeks ago

obriat commented 1 month ago

Close #2931

cyberw commented 1 month ago

Can you do this without adding the humanfriendly dependency? Seems a bit much to add an entire package just to format one date...

There's already some info about debugging in running-in-debugger.rst, can you maybe add your info there instead (and link to it from developing-locust.rst)

@andrewbaldwin44 Any opinions on the TS stuff?

andrewbaldwin44 commented 1 month ago

@obriat Could you also check that this works with the --html flag?

obriat commented 1 month ago

Can you do this without adding the humanfriendly dependency? Seems a bit much to add an entire package just to format one date...

I'm not a Python expert, but does this package adds a overhead to the runners or just the report part?

There's already some info about debugging in running-in-debugger.rst, can you maybe add your info there instead (and link to it from developing-locust.rst)

I'll move my doc to this page

JavierUhagon commented 1 month ago

I'm not a Python expert, but does this package adds a overhead to the runners or just the report part?

Hey there! Not a mantainer, but, I don't think the issue is adding a overhead, it's having to add a full package (that hasn't been updated since 2020) just to format a date for a report, I'd say that adding your own implementation would be better :)

You can always check how humanfriendly does it! https://github.com/xolox/python-humanfriendly/blob/6758ac61f906cd8528682003070a57febe4ad3cf/humanfriendly/__init__.py#L402

It really isn't hard nor long!

obriat commented 3 weeks ago

@obriat Could you also check that this works with the --html flag? Hi, I think that I response to every suggestions and fix failed tests.

About the --html flag, this option needs a filename as input (not mandatory but throw an exception on writing file). So what should we do ?

andrewbaldwin44 commented 3 weeks ago

Hey @obriat, for the --html <filename> flag, I just meant to double-check it's still working as expected, since it also uses the get_html_report function that was modified, but looks fine so no need to change anything :+1:

cyberw commented 2 weeks ago

I tested --html just for fun :) It doesn't seem to have the duration?

image

Once you've fixed it, maybe add a search for ' seconds' here to ensure it works https://github.com/locustio/locust/blob/eba774c8db030a103fd82bcdf26cef0d5376415e/locust/test/test_main.py#L1183

cyberw commented 2 weeks ago

Btw, resolve the conflicts in launch.json whatever way you like, dont worry about my changes :)

obriat commented 2 weeks ago

I’ll fix the conflict, a bit strange about the —html I’m pretty sure I tested it :( Is there a way to make a automated test about it?

obriat commented 2 weeks ago

I tested --html just for fun :) It doesn't seem to have the duration?

"It works on my PC":

git branch --show-current
report-name
poetry run locust -f examples/basic.py  -u 1 -H http://192.168.64.100:8089 --headless -u 1 -t 30s --html ~/headless-locust-report.html
grep -Eo '"duration": "[^"]*"'  ~/headless-locust-report.html
"duration": "29.067671298980713 seconds"

The rendered html is also correct "During: 03/11/2024 23:01:02 - 03/11/2024 23:01:31 (29.067671298980713 seconds)"

(Yes, I know, I got a round problem 😄 )

cyberw commented 2 weeks ago

Yea sorry, missing the seconds was my bad, I must have messed up rebuilding the UI.

Test looks good, but we dont have pytest as a dependency so looks like it is failing. I think we'll be going in the pytest direction in not too long a while, so feel free to add it to the dev dependencies (something like poetry add --group dev pytest)

obriat commented 2 weeks ago

Done, pyproject.toml changes are readable, but I don't know about the poetry.lock ones.

cyberw commented 2 weeks ago

Sorry, it was the wrong group, it was supposed to be added to "test", not "dev". I'll fix it in master. Thanks!