fedimint / fedimint

Federated E-Cash Mint
https://fedimint.org/
MIT License
536 stars 210 forks source link

chore: put tighther deadlines on test runtime #5175

Closed dpc closed 1 week ago

dpc commented 2 weeks ago

Will rebase after #5176 lands

This is to prevent silent regressions.

I wish we could go lower. Most devimint-based tests are already below 40s, and unit-test like tests should be OK under 20s, IMO.

We'll get there.

bradleystachurski commented 2 weeks ago
TIMEOUT [  60.007s] fedimint-wallet-tests::fedimint_wallet_tests peg_out_fail_refund

Perhaps a bit too tight?

bradleystachurski commented 2 weeks ago

Timing out now with the updated parallel 130s timeout

       > 36    :       1714530498.663     130.344      0       28      -1      15      run_test_for_versions bckn_gw_not_client FM: current CLI: current GW: current
       >
       > 63      :       1714530587.481     130.960      0       28      -1      15      run_test_for_versions bckn_gw_not_client FM: current CLI: current GW: current

Perhaps we can resolve that one by breaking up the gateway tests into smaller chunks?

dpc commented 2 weeks ago

Perhaps we can resolve that one by breaking up the gateway tests into smaller chunks?

I already did and split it in two.

It's OK. I'll increase to 150.

maan2003 commented 2 weeks ago

IMO time limit should be defined per test

dpc commented 2 weeks ago

IMO time limit should be defined per test

Could be done as an additional one, but seems like somewhat manual thing to maintain.

dpc commented 2 weeks ago

:crossed_fingers: This change is cursed and keeps triggering new unseen yet flakes.

bradleystachurski commented 2 weeks ago

This change is cursed

I'm becoming concerned that if we land this, subsequent PRs will have to get lucky to pass the merge queue. Should we hold off?

dpc commented 2 weeks ago

I'm becoming concerned that if we land this, subsequent PRs will have to get lucky to pass the merge queue. Should we hold off?

If we're hitting tests legitimately taking more time the deadlines we should raise them. But if it's unrelated flakes (which seems to have happened a lot here), holding off doesn't help.

dpc commented 2 weeks ago

Last failure might be fixed by #5179 so will re-add to MQ after it lands.

m1sterc001guy commented 2 weeks ago

Lightning tests will get faster in the coming weeks as I consolidate them into devimint tests and remove running them twice for each gateway.