search

microsoft / msquic

Cross-platform, C implementation of the IETF QUIC protocol, exposed to C, C++, C# and Rust.
MIT License
4.05k stars 530 forks source link

Linux tests should cover more than Ubuntu Linux #3460

Closed wfurt closed 1 month ago

wfurt commented 1 year ago

Describe the bug

It seems like current test coverage only covers ubuntu-latest. The is problematic IMHO as OpenSSL differs on different distributions e.g. there is more than just OpenSSL 1.1 and 3.

We should probably cover at least following distributions as addition

This probably does not need to be full set. Platforms tests + some basic coverage should be sufficient IMHO

One can also take inspiration from https://github.com/dotnet/core/blob/main/release-notes/7.0/supported-os.md

Affected OS

Additional OS information

No response

MsQuic version

main

Steps taken to reproduce bug

run MsQuic on various linux distributions

Expected behavior

quic should load and function

Actual outcome

quic sometimes fails to load because of differences in OpenSSL library

Additional details

cc: @manickap @CarnaViire

CarnaViire commented 1 year ago

We may also use this as an integration test platform between MsQuic and .NET. I've made some scripts to check the integration on minimalistic Docker images covering most of https://github.com/dotnet/core/blob/main/release-notes/8.0/supported-os.md -- the test installs the latest .NET daily build and MsQuic from packages.microsoft.com on Docker images and checks whether the lib at least loads to .NET properly (QuicConnection.IsSupported == true). It can be expanded to include some basic echo test.

I might be able to poke at this after we're done with .NET 8. I believe we wanted to have some integration testing within MsQuic CI pipeline even before, but we tried to approach it from a too complicated side.

liveans commented 1 month ago

We've done some work related to this in #4466, but should we close this as resolved or more work is necessary (like running unit tests etc. in different containers)?

/cc @wfurt @nibanks

CarnaViire commented 1 month ago

Should we also add Alpine? @liveans I believe it was not part of https://github.com/microsoft/msquic/pull/4466 (and it's also mentioned here in the issue description).

liveans commented 1 month ago

Should we also add Alpine? @liveans I believe it was not part of #4466 (and it's also mentioned here in the issue description).

It was part of #4554, and completed recently, the only missing part for Alpine is ARM32, because dotnet is not working reliably over simulation on ARM32 Alpine.

CarnaViire commented 1 month ago

Then it's all good, thanks! IMO we can consider it done, and open a new issue if we would like to expand further.

nibanks commented 1 month ago

Thanks so much for all the help on this!