Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test1022: Add support for rc releases #16626

Closed
wants to merge 1 commit into from

Conversation

samueloph
Copy link
Contributor

@samueloph samueloph commented Mar 8, 2025

Fix the following test failure:
curl-config: illegal value

Note: The rc relase tarball is currently named with the -rc suffix but the --version parameter reports it as -rc1.
Usually, the lack of a number implies that -rc is equivalent to -rc0.
This does not cause any issues today, but I recommend matching these numbers for the next releases (and to assume rc == rc0).

 Fix the following test failure:
 curl-config: illegal value
@testclutch
Copy link

Analysis of PR #16626 at 87d563a1:

Test 564 failed, which has NOT been flaky recently, so there could be a real issue in this PR.

Test 713 failed, which has NOT been flaky recently, so there could be a real issue in this PR.

Test 715 failed, which has NOT been flaky recently, so there could be a real issue in this PR.

Generated by Testclutch

@samueloph
Copy link
Contributor Author

Analysis of PR #16626 at 87d563a1:

Test 564 failed, which has NOT been flaky recently, so there could be a real issue in this PR.

Test 713 failed, which has NOT been flaky recently, so there could be a real issue in this PR.

Test 715 failed, which has NOT been flaky recently, so there could be a real issue in this PR.

Generated by Testclutch

Logs:

FAIL 564: 'FTP RETR a file over a SOCKS proxy using the multi interface' FTP, PASV, RETR, multi, SOCKS4
FAIL 713: 'FTP fetch with --proxy set to socks5:// and with --connect-to' FTP, PASV, RETR, SOCKS5, CURLOPT_CONNECT_TO
FAIL 715: 'FTP fetch with --preproxy, --proxy and --connect-to' FTP, PASV, RETR, HTTP, HTTP CONNECT, proxytunnel, SOCKS5, CURLOPT_CONNECT_TO

TESTFAIL: These test cases failed: 564 713 715

These tests are not related to this PR.

@thesamesam
Copy link
Contributor

thesamesam commented Mar 8, 2025

Hm, is there no distcheck job in CI? It should catch this.

EDIT: Ah, there's a job called distcheck (https://github.com/curl/curl/blob/2fc8f7a3f74f2f07171350df4945f01de58f19ee/.github/workflows/distcheck.yml), but it doesn't actually use make distcheck.

@dfandrich
Copy link
Contributor

dfandrich commented Mar 8, 2025 via email

@thesamesam
Copy link
Contributor

thesamesam commented Mar 8, 2025

OK, so not the same thing ;)

i.e. autotools distcheck exists for this exact case, to ensure that both files are not missing from a tarball, and that the versions within the tarball work fine for building/tests/installation.

EDIT: #12088 (comment)

@bagder
Copy link
Member

bagder commented Mar 8, 2025

We do that exact discheck in CI. That's not the problem. The problem was that we never did a -rc build before, and the distcheck in CI does not either.

@bagder bagder closed this in 3c1a88f Mar 8, 2025
@bagder
Copy link
Member

bagder commented Mar 8, 2025

Thanks!

@thesamesam
Copy link
Contributor

It's not the same check if it doesn't use the same version as whatever the tag is (it uses 99.98.97). But I see there isn't a tag for the RC either.

@bagder
Copy link
Member

bagder commented Mar 8, 2025

What is your point?

@thesamesam
Copy link
Contributor

If a tag was made for the RC and it used the tag value if existed, it'd catch this? Right now, it uses a fixed version, so if there's something subtle like this test, it's not going to notice.

@samueloph samueloph deleted the samueloph/test1022_rc_releases branch March 9, 2025 10:18
@bagder
Copy link
Member

bagder commented Mar 9, 2025

If we updated the test so that it would test the scenario that it didn't work for, then it would have caught this problem. Yes, that seems correct. We welcome all help and assistance we can get in improving our test suite.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants