nixos/test/ipv6: Specify port in curl #51274
Conversation
GrahamcOfBorg
added
6.topic: nixos
There was a problem hiding this comment.
While testing curl is out of scope for the IPv6 test, it has however caught a real bug here (which according to this comment might the reason for the cherry-pick from upstream). So instead of adding workarounds for a thing that will cease to be a problem once #50905 lands in master I'd rather opt to either not merge this or make sure this will be reverted once ed08ff4 hits master. One way could be to add another $client->fail without the port number to make sure the test fails again once the commit lands in master.
|
Another way would be to add an assertion like: assert !lib.any (lib.hasSuffix "fix-ipv6-url-parsing.patch") curl.patches; ...... however that might be racy if a new upstream version of |
|
@aszlig I agree that it is a good thing that the test caught a bug (albeit unexpectedly), but even nixos-unstable-small channel updates were delayed due to this failure, at least for a week if I recall correctly. (Off-topic: is there a way to show complete history, e.g. https://hydra.nixos.org/job/nixos/unstable-small/tested#tabs-constituents show only the last builds). I also agree with you that adding the assertion would be racy, no need to have it on this test. If the majority still wants to test curl functionality, I will be in favor of closing this PR. |
There was a problem hiding this comment.
Yes, well I'd say I agree with aszlig, but it's certainly no strong opinion in my case.
The particular bug that broke it recently was perhaps considered not significant enough to block the small channel, but it has been unblocked in the meantime and it seems unlikely it will occur again (in a similar-enough way to be resolved by this PR).
|
Are there any updates on this pull request, please? |
|
Given that the test passes in https://hydra.nixos.org/eval/1583095?filter=ipv6&compare=1582978&full= I'm closing this |
Motivation for this change
A curl update has recently broken the IPv6 test. #50905 fixes curl and eventually the test, but the commits are on staging-next and I would like the IPv6 test to test IPv6 functionality only, so I propose to make the curl commands more defined anyway, i.e. to include the port number.
Things done
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)nix path-info -Sbefore and after)