-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Update delete integration test #6192
Update delete integration test #6192
Conversation
Signed-off-by: Michel Hollands <[email protected]>
Signed-off-by: Michel Hollands <[email protected]>
Signed-off-by: Michel Hollands <[email protected]>
Signed-off-by: Michel Hollands <[email protected]>
Signed-off-by: Michel Hollands <[email protected]>
Signed-off-by: Michel Hollands <[email protected]>
Signed-off-by: Michel Hollands <[email protected]>
Signed-off-by: Michel Hollands <[email protected]>
Signed-off-by: Michel Hollands <[email protected]>
Signed-off-by: Michel Hollands <[email protected]>
Signed-off-by: Michel Hollands <[email protected]>
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester 0%
+ distributor 0%
+ querier 0%
+ querier/queryrange 0%
+ iter 0%
+ storage 0%
+ chunkenc 0%
+ logql 0%
+ loki 0% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks awesome
Signed-off-by: Michel Hollands <[email protected]>
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester 0%
+ distributor 0.3%
+ querier 0%
+ querier/queryrange 0%
+ iter 0%
+ storage 0%
+ chunkenc 0%
+ logql 0%
+ loki 0% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! LGTM
Signed-off-by: Michel Hollands <[email protected]>
./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell. + ingester 0%
+ distributor 0.3%
+ querier 0%
+ querier/queryrange 0%
+ iter 0%
+ storage 0%
+ chunkenc 0%
+ logql 0%
+ loki 0% |
newErr := fmt.Errorf("error starting component %v: %w", c.name, err) | ||
errCh <- newErr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: since newErr
is used nowhere else ...
newErr := fmt.Errorf("error starting component %v: %w", c.name, err) | |
errCh <- newErr | |
errCh <- fmt.Errorf("error starting component %v: %w", c.name, err) |
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-6192-to-k102 origin/k102
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x 65e3148bc97159a2efaefdc0bcc5e2fa88c40870
# Push it to GitHub
git push --set-upstream origin backport-6192-to-k102
git switch main
# Remove the local backport branch
git branch -D backport-6192-to-k102 Then, create a pull request where the |
* Add wait until processed Signed-off-by: Michel Hollands <[email protected]> * Add query lines Signed-off-by: Michel Hollands <[email protected]> * Remove Cortex reference Signed-off-by: Michel Hollands <[email protected]> * Add deleted lines metric and check in test Signed-off-by: Michel Hollands <[email protected]> * Set negative cancel period so default 1 minute delay is compensated for Signed-off-by: Michel Hollands <[email protected]> * Reduce timeout Signed-off-by: Michel Hollands <[email protected]> * Add helper function Signed-off-by: Michel Hollands <[email protected]> * Comment out assertions that only work with flush Signed-off-by: Michel Hollands <[email protected]> * Remove unused metric type param Signed-off-by: Michel Hollands <[email protected]> * Check counter in unit test Signed-off-by: Michel Hollands <[email protected]> * Rename ClientOption to Option as per the linter Signed-off-by: Michel Hollands <[email protected]> * Remove redundant check Signed-off-by: Michel Hollands <[email protected]> * fill in MsgAndArgs field of test assertion Signed-off-by: Michel Hollands <[email protected]> (cherry picked from commit 65e3148)
What this PR does / why we need it:
Update the deletion integration test so it waits until the delete request has been processed. Note that at the moment no lines are removed due to flush not working. This will be fixed later on in a separate PR.
Add a metric to show how many lines are deleted.
Remove mention of Cortex in a helper function.
Change the various timeouts so the test runs in 10 seconds.
Add code to parse and check metrics.
Checklist
CHANGELOG.md
.docs/sources/upgrading/_index.md