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

Fix flaky reorder regression test take 2 #7538

Merged
merged 1 commit into from
Dec 16, 2024

Conversation

fabriziomello
Copy link
Contributor

In #7509 we added manual VACUUM in all involved relations to avoid flaky output tests.

But looks like it is not working very well specially on Windows builds, so now we're removing the output found 0 removable, 0 nonremovable row versions in 0 pages to have a more predictable output test.

https://github.com/timescale/timescaledb/actions/runs/12322325859/job/34395659212?pr=7533#step:17:24

Disable-check: force-changelog-file
Disable-check: approval-count

@fabriziomello fabriziomello added testing flaky-test Issue about a flaky test labels Dec 13, 2024
@fabriziomello fabriziomello self-assigned this Dec 13, 2024
Copy link

@mkindahl, @antekresic: please review this pull request.

Powered by pull-review

Copy link

codecov bot commented Dec 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.16%. Comparing base (59f50f2) to head (74de124).
Report is 657 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7538      +/-   ##
==========================================
+ Coverage   80.06%   82.16%   +2.09%     
==========================================
  Files         190      230      +40     
  Lines       37181    43280    +6099     
  Branches     9450    10882    +1432     
==========================================
+ Hits        29770    35559    +5789     
- Misses       2997     3399     +402     
+ Partials     4414     4322      -92     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@mkindahl mkindahl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want you can reduce the min messages to WARNING in the move test, since that is not relevant for the test.

For the other cases I agree that the informational messages needs to be printed (to show progress of the operation), and that the "rebuild table" information is not necessary for the tests, so approving.

test/expected/cluster-14.out Show resolved Hide resolved
tsl/test/expected/move.out Outdated Show resolved Hide resolved
tsl/test/expected/reorder.out Show resolved Hide resolved
@fabriziomello fabriziomello force-pushed the fix_flaky_reorder_take2 branch 2 times, most recently from 14dfbc2 to 3a4bca6 Compare December 16, 2024 18:01
In timescale#7509 we added manual VACUUM in all involved relations to avoid
flaky output tests.

But looks like it is not working very well specially on Windows builds,
so now we're removing the output `found 0 removable, 0 nonremovable
row versions in 0 pages` to have a more predictable output test.

https://github.com/timescale/timescaledb/actions/runs/12322325859/job/34395659212?pr=7533#step:17:24
@fabriziomello fabriziomello force-pushed the fix_flaky_reorder_take2 branch from 3a4bca6 to 74de124 Compare December 16, 2024 20:56
@fabriziomello fabriziomello enabled auto-merge (rebase) December 16, 2024 21:02
@fabriziomello fabriziomello merged commit 12f262c into timescale:main Dec 16, 2024
50 of 51 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flaky-test Issue about a flaky test testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants