search

Xilinx-CNS / tcpdirect

AMD TCPDirect ultra low latency kernel bypass TCP and UDP implementation for AMD Solarflare network adapters, to be used with corresponding versions of Onload®️ at https://github.com/Xilinx-CNS/onload. The stable branch is currently `v8_1`.
22 stars 17 forks source link

ON-15724: improve unit test consistency #20

Closed jfeather-amd closed 6 months ago

jfeather-amd commented 6 months ago

These changes improve the consistency of the unit tests, mainly those under the test_jenkins target.

The only test that still fails after this patchset is zftimestamping, but this appears to be fixed by Xilinx-CNS/tcpdirect#15. For now, I will keep these tests as part of the test_jenkins target until I'm more satisfied that these fixes work after getting more test exposure over time.

Testing Done

Repeated lots of iterations of make test and make test_jenkins to observe behaviour over time. Previously, the failing testscripts would fail every 10 or so runs of make test_jenkins but I have not observed a single failure in quite a lot of runs. This is a very hand-wavy testing done section, which is why I would like to observe these changes over time before committing to saying that these are fine.

jfeather-amd commented 6 months ago

Looks like zfstackdump is perhaps a github setup issue, so I'll look into what causes that.

jfeather-amd commented 6 months ago

Latest push rebases to also include Andy's fix for timestamping and also fixes the issue with zfstackdump. Honestly not entirely sure why the old method didn't work, but I guess Ubuntu just wasn't a fan of taking input with <&1000 perhaps because that was only known to us and not the child process? Anyway, tested the new approach on Ubuntu 22.04 and RHEL 8.6, both seem happy.

sianj-xilinx commented 6 months ago

Honestly not entirely sure why the old method didn't work, but I guess Ubuntu just wasn't a fan of taking input with <&1000 perhaps because that was only known to us and not the child process?

We've seen things like this in the past where ubu defaulted to dash not bash. I haven't looked at the context, but that sort of this might be the case here.