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

tests: handle accepted regressions in release builds #23984

Conversation

nrainer-materialize
Copy link
Contributor

@nrainer-materialize nrainer-materialize commented Dec 18, 2023

A known performance and scalability regression was introduced in v0.80.0. While regular nightly builds did not fail because because the regressions were marked as accepted, this was not the case for the release build.
This PR considers known regressions when running nightly builds for releases, which are based on versions.

Commits

cfce28b docker: add utility methods to classify image tag
6ceb50c tests: identify accepted regressions between two versions
89cafdb scalability framework: refactorings: extract methods
244a70a scalability framework: store resolved target of endpoints
bf2fa67 scalability framework: collect endpoints with regressions
1d8b4c5 scalability framework: print assessment, do not fail on justified regressions
c2f222c feature benchmark: refactoring: variable usages
4cbfcd7 feature benchmark: print assessment, do not fail on justified regressions

Status

Draft, need to go over the code myself again.
Please pay special attention to c2f222c, which seems risky.

Nightlies

@nrainer-materialize nrainer-materialize added the T-testing Theme: tests or test infrastructure label Dec 18, 2023
@nrainer-materialize nrainer-materialize self-assigned this Dec 18, 2023
@nrainer-materialize nrainer-materialize force-pushed the tests/handle-accepted-regressions-in-release-builds branch 3 times, most recently from 110ad9d to acf11ca Compare December 19, 2023 10:22
@nrainer-materialize nrainer-materialize marked this pull request as ready for review December 19, 2023 10:23
@nrainer-materialize nrainer-materialize force-pushed the tests/handle-accepted-regressions-in-release-builds branch from acf11ca to e6ccb7e Compare December 20, 2023 09:02
@nrainer-materialize
Copy link
Contributor Author

Nightly comparing v0.80.0 with v0.79.1 as baseline worked as expected:
✅ Builds succeeded
✅ Scalability test prints:

+++ Assessment of regressions
* Although there are regressions between baseline v0.79.1 (38a21b1a6) - release: bump to version v0.79.1 and endpoint v0.80.0 (15e58e51d) - release: bump to version v0.80.0, they can be explained by the following commits that are marked as accepted regressions: c4f520a57a3046e5074939d2ea345d1c72be7079.

✅ Feature benchmark prints:

ERROR: The following scenarios have regressions: InsertMultiRow, ConnectionLatency, SubscribeParallelTableWithIndex, StartupLoaded, SubscribeParallelKafka
However, the regressions are accepted. Accepted regressions were introduced with these commits: ['c4f520a57a3046e5074939d2ea345d1c72be7079']

@nrainer-materialize nrainer-materialize force-pushed the tests/handle-accepted-regressions-in-release-builds branch from e6ccb7e to 2590803 Compare December 20, 2023 09:12
@nrainer-materialize nrainer-materialize force-pushed the tests/handle-accepted-regressions-in-release-builds branch from 2590803 to 4cbfcd7 Compare December 20, 2023 09:16
@nrainer-materialize nrainer-materialize merged commit 35a534c into MaterializeInc:main Dec 20, 2023
67 checks passed
@nrainer-materialize nrainer-materialize deleted the tests/handle-accepted-regressions-in-release-builds branch December 20, 2023 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-testing Theme: tests or test infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants