Skip to content
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

Performance analysis for tests #4988

Closed
webzwo0i opened this issue Apr 2, 2021 · 2 comments
Closed

Performance analysis for tests #4988

webzwo0i opened this issue Apr 2, 2021 · 2 comments
Labels
Feature Request wontfix Wont Fix these things, no hate.

Comments

@webzwo0i
Copy link
Member

webzwo0i commented Apr 2, 2021

With 4ad80d4 (from PR #4987) a lot of short timeouts got removed from frontend tests. This helps with test stability. However, in the past, at least that's my impression, people thought of those timeouts to be some kind of guard against performance degregations, ie if some editor causes slowness, it was hoped that short timeouts gonna catch it.

I don't think this assumption was entirely correct. There is few code that is guaranteed to be processed immediately and if we use timeouts and tests fail we cannot be sure if it's due to us having introduced some slowness or due to some browser/selenium change, mocha etc. Besides, timeouts are set for a whole test or suite, and a test is using multiple lines of code with DOM, events, network i/o etc. so we cannot be certain what code is running too slow.

Often this lead to commits that increase timeouts, as we saw the code is fast in some browsers, while slightly slower in others, which is again contrary to the assumption that we get info about performance regressions when tests fail due to timeouts.

I think we should rather start analyzing the build logs on a per browser basis and visualize the duration of every single test and/or use some simple average. It would be great if those analysis is done as another job, so that instead of failing a test due to timeouts, we would fail the performance analysis job. However, if for a start it would be enough to somehow visualize the deriviation from the average/expectation

Update:
imo it's useful both for backend and frontend tests

@rhansen
Copy link
Member

rhansen commented Apr 2, 2021

I completely agree. Fixed timeouts don't adjust for hardware performance differences, so even if there isn't noise in the system, they'll be too long on fast machines and too short on slow machines.

I suspect that there will be too much variance for test suite timing to be useful. Also, we have a lot of sleeps in our tests that hide the true cost. Ideally we would have a benchmark suite that we could use for before/after comparisons on each pull request, but that would take a lot of work to create and maintain. If Etherpad is too slow, it's likely easier and more useful to add sharding, split into microservices, or just throw more hardware at the problem.

@stale
Copy link

stale bot commented Jun 2, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request wontfix Wont Fix these things, no hate.
Projects
None yet
Development

No branches or pull requests

2 participants