Fix flaky tests on the ci

[sigmaSd]

This is just an issue to remind us and to get other people to look into this.

Currently tests are flaky on the ci (they mostly fail):

• is it because of the interaction of tui / cargo insta ? (ratatui test suite seems to be passing)
• is it because of the nature of banwhich maybe network values are different (unlikely)

If we could pinpoint the most flaky tests and disable those I think that would be a good starting point. We can always re-enable stuff gradually after.

[cyqsimon]

The mock packets in the tests are all static, hand-crafted values. So I think it's definitely due to some funkiness in ratatui. If necessary I can go ask their maintainers for assistance directly.

---

I'm also thinking, maybe testing the terminal output (and events) equality to a T is overly strict and frankly silly. Is there perhaps a better way to test the general functionality of the TUI without necessarily demanding strict equality?

[cyqsimon]

Take a look at the CI runs in #305. They are starting to pass consistently (well, somewhat consistently) without me specifically tackling them.

I think the issue was that we were testing right at the edges of "layout switches", and the terminal emulator (or whatever silly component it is) had some kind of spontaneous and intermittent off-by-one error somewhere. Now that I've completely changed the "layout switch edges" (what I called "cutoff points" in code), these off-by-one errors are no longer getting triggered.

Frankly speaking we can just leave it here and call it a day. But the perfectionist in me still wants to find a better way to test this. as mentioned in my previous post.

[cyqsimon]

Well, I found one of the problems (maybe the only problem even). It's the god damn timestamp on the upper-right corner in cumulative mode. Depending on how fast or slow the test runs, obviously the output will be different. This also explains why I could not reproduce these CI failures locally.

Take a look at the uploaded snapshots in this CI run.

[sigmaSd]

Awesome work @cyqsimon hopefully we finally get a green ci

[sigmaSd]

! checked the last ci runs failure, its unclear to me why they fail there doesn't seem a failure in a test or even an error ?

Do you have an idea about this ?

[cyqsimon]

I think I have a decent grasp on the root cause of the situation. See my own replies underneath #308.

The best way forward is most certainly a big refactor, which is forthcoming™️. Give me a bit of time.

[sigmaSd]

Thanks for your work @cyqsimon !

[YJDoc2]

Hey @cyqsimon I saw there are still some issues with whole CI ; there are flaky tests as well as some clippy issues for certain platform/toolchains. I'd like to try and help, but wanted to check in once before I start if you have something planned/being worked on or do you want to do this in some certain way.

[cyqsimon]

Hey @cyqsimon I saw there are still some issues with whole CI ; there are flaky tests as well as some clippy issues for certain platform/toolchains.

Yeah we've spent a whole lot of time figuring out why tests are so flaky. The reason comes down to a race condition at program termination. The way the main loop is written now, it's possible for an extra frame to be drawn before the process exits, which causes the test output to differ from the expected value stored in snapshots.

The effect is mostly harmless, apart from the tests failing. However I have noticed that sometimes the extra frame can be drawn after the terminal has left the alternate screen state, creating some garbage on the terminal. In the worst case this can require a reset from the user.

Regardless, yeah this is something I do plan to fix. I've already done some major restructuring of the main loop to use message passing, but there are still lots of todo!()s left. As stated in #275, I haven't got an abundance of time, so progress has been slow.

---

As of the clippy issue, I did take a look today. It's just a newly promoted lint in nightly Rust causing problems so nothing serious. I partially addressed it in #432, but a few warnings actually originate from the expanded code of derivative's derive macros (see cargo expand display::components::display_bandwidth), so there's nothing we can do about that.

Unfortunately the crate seems abandoned, so we can't even submit a PR. Moving to something like derive_more seems like a good idea, except it doesn't offer this:

bandwhich/src/cli.rs

Lines 36 to 38 in 4faccd1

	#[command(flatten)]
	#[derivative(Default(value = "Verbosity::new(0, 0)"))]
	pub verbosity: Verbosity<InfoLevel>,

Granted it's not a big deal, just have to do a manual impl Default instead, but I'm picky like that. 😎 Ideally I want to switch to an active fork if exists, or at least something that has feature parity.

---

I'd like to try and help, but wanted to check in once before I start if you have something planned/being worked on or do you want to do this in some certain way.

Thanks for offering; we can indeed use the help. I was originally planning to do the refactor alone, so I haven't bothered to push it. But since you want to help I'll prepare a WIP branch for it. Will ping you when ready.

[YJDoc2]

Unfortunately the crate mcarton/rust-derivative#117, so we can't even submit a PR. Moving to something like derive_more seems like a good idea, except it doesn't offer this:

Just checking off the comments on that issue, https://github.com/magiclen/educe/ might be another alternative? I haven't looked at the code of bandwhich in detail, but from the readme of educe , it supports similar options on debug and default derives. I can try to see if bandwhich can use it instead of derivative if you're ok with educe and would be willing to accept a PR of change if it works out.

Granted it's not a big deal, just have to do a manual impl Default instead, but I'm picky like that. 😎

😂 ✌️

I was originally planning to do the refactor alone, so I haven't bothered to push it. But since you want to help I'll prepare a WIP branch for it. Will ping you when ready.

Glad to help if I can! Also if it is going to be more work for you to make a WIP branch, and you feel that at this stage it is better if you develop this part alone, that is fine too. I just saw that the CI was failing, and this repo could is accepting contributions, so commented.

[cyqsimon]

Just checking off the comments on that issue, https://github.com/magiclen/educe/ might be another alternative? I haven't looked at the code of bandwhich in detail, but from the readme of educe , it supports similar options on debug and default derives. I can try to see if bandwhich can use it instead of derivative if you're ok with educe and would be willing to accept a PR of change if it works out.

Yeah it looks like it will work. PR welcomed!

Also if it is going to be more work for you to make a WIP branch, and you feel that at this stage it is better if you develop this part alone, that is fine too.

Not at all. Any additional input, let it be ideas, code, review, second opinions or whatever is greatly appreciated, especially given my very meagre time constraints. It also helps that I'm no longer that kid who thinks it's my way or the highway 😅 (at least I'd like to think so).