istio / ztunnel

The `ztunnel` component of ambient mesh
Apache License 2.0
287 stars 96 forks source link

Access log tests are not useful when running tests concurrently #1084

Open howardjohn opened 3 months ago

howardjohn commented 3 months ago

https://github.com/istio/ztunnel/blob/80ac30e0bb69c32c07d1ca5fdebfaa084ed6b11d/tests/namespaced.rs#L995

rust runs tests concurrently by default. We may get false-positives by matching other tests logs instead of our tests.

--test-threads 1 or cargo-nextest can weed these out, but it would be nice to somehow have per-test log collection. Note sure how though.

FWIW I always use cargo-nextest so as long as I am running tests locally we will find these issues quickly. However, that is a terrible long term stance :slightly_smiling_face:

bleggett commented 3 months ago

Can we just use nextest in CI? I don't see why not.

howardjohn commented 3 months ago

We could, although that also leaves the inverse possible broken test (only works when not run concurrently)... either way we lose. Using nextest is good for other reasons though, probably a decent choice. We will need to add to build tools though

bleggett commented 3 months ago

We could, although that also leaves the inverse possible broken test (only works when not run concurrently)... either way we lose.

Fine with that tradeoff, as those sorts of tests would almost assuredly show up as flakes in concurrent runs anyway.

bleggett commented 3 months ago

We should be able to use cargo test -- --shuffle eventually to flag flakes faster without nextest - currently that's behind unstable/nightly tho, better to wait for that.