tests/test_appender.py::test_appending_beats_inserting_by_the_margin_that_makes_it_worth_having asks for a five times margin over 200 rows and gets between two and four on a GitHub runner. It has failed on both ubuntu 3.11 and ubuntu 3.14 on every run of main since #30, and it fails on every branch, so it is not telling anyone anything about the branch under it.
The last two runs, both of them main:
200 rows: 24 ms inserted, 7 ms appended ubuntu 3.11
200 rows: 16 ms inserted, 6 ms appended ubuntu 3.14
The margin is real and the measurement is not. 200 rows is tens of milliseconds, a shared runner has other tenants, and a test that divides one noisy number by another noisy one is measuring the neighbours. What the test is for is worth keeping: the appender exists because it is faster than a statement per row, and a change that lost that should fail something.
Two ways out and they are not exclusive. Raise the row count until the numbers are large enough that scheduling noise is a rounding error, which costs seconds of CI rather than milliseconds. Or keep the assertion and stop counting a fixed margin: assert that appending is faster at all in CI, and keep the five times figure for a benchmark that runs where a benchmark can be believed.
Until then it is red on every pull request, which is worse than not running, because a gate everyone has learned to read past is a gate that will miss the real one.
tests/test_appender.py::test_appending_beats_inserting_by_the_margin_that_makes_it_worth_havingasks for a five times margin over 200 rows and gets between two and four on a GitHub runner. It has failed on both ubuntu 3.11 and ubuntu 3.14 on every run of main since #30, and it fails on every branch, so it is not telling anyone anything about the branch under it.The last two runs, both of them main:
The margin is real and the measurement is not. 200 rows is tens of milliseconds, a shared runner has other tenants, and a test that divides one noisy number by another noisy one is measuring the neighbours. What the test is for is worth keeping: the appender exists because it is faster than a statement per row, and a change that lost that should fail something.
Two ways out and they are not exclusive. Raise the row count until the numbers are large enough that scheduling noise is a rounding error, which costs seconds of CI rather than milliseconds. Or keep the assertion and stop counting a fixed margin: assert that appending is faster at all in CI, and keep the five times figure for a benchmark that runs where a benchmark can be believed.
Until then it is red on every pull request, which is worse than not running, because a gate everyone has learned to read past is a gate that will miss the real one.