ci: add ci to prevent future pr's breaking sim #124
Conversation
KolbyML
force-pushed
the
add-test
branch
3 times, most recently
from
ee967c6 to
5cc8ced
Compare
|
@marten-seemann ok the PR is ready for review. I tested it on master and it failed as expected. I also tested it with the Fix Lars summited and the test passed as expected. The test is very minimal in scope, but it prevents the event that happened yesterday so it does its job. Cheers! |
KolbyML
force-pushed
the
add-test
branch
3 times, most recently
from
d8fad24 to
7b57861
Compare
marten-seemann
left a comment
marten-seemann
left a comment
There was a problem hiding this comment.
Thank you @KolbyML for working on this!
Building the simulator takes a long time (~1h) on CI. I'd prefer if we could make this an additional step in the build-and-push workflow (i.e. first build, then test, then push), instead of adding a new workflow, for 2 reasons:
- obviously, build time
- it's safest to test exactly the image that we're about to push to DockerHub
|
@marten-seemann it only takes Also yeah for sure that is no issue! |
|
Huh, this is weird. What's going on here? Compiling ns3 from scratch takes more than an hour, see https://github.com/quic-interop/quic-network-simulator/actions/runs/7465935034/job/20328138279?pr=124 |
|
Maybe that is being caused by qemu? I did docker build -f sim/Dockerfile . and it was always taking less then 7 minutes on ci I was testing it all morning |
|
In that case I don't think we should combine the CI I am going to rebase the pr to show it becomes green with Lars fix |
|
Here is a picture taken on broken master for future reference |
|
@marten-seemann you have to approve the CI to run. But anyways it will be successful and run in 7-8 minutes which is not 90 minutes so I don't think we should merge the 2 CI's together. QEMU do be slow though |
2 participants
No description provided.