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

tests: re-enable 2086, and 472, 1299, 1613 for Windows #15644

Closed
wants to merge 5 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Nov 26, 2024

@vszakats vszakats added the tests label Nov 26, 2024
@vszakats vszakats marked this pull request as draft November 26, 2024 14:09
They are failing on Linux/macOS/Windows, 1801 consistently,
1184 a bit less consistently.
@github-actions github-actions bot added the CI Continuous Integration label Nov 26, 2024
@vszakats vszakats changed the title DISABLED: re-enable 1184 1510 1801 2086 tests: re-enable some Nov 26, 2024
@vszakats vszakats marked this pull request as ready for review November 26, 2024 15:21
@vszakats vszakats changed the title tests: re-enable some tests: re-enable test 2086 and 472, 1299, 1613 for Windows. Nov 26, 2024
@vszakats vszakats changed the title tests: re-enable test 2086 and 472, 1299, 1613 for Windows. tests: re-enable 2086 and 472, 1299, 1613 for Windows. Nov 26, 2024
@vszakats vszakats changed the title tests: re-enable 2086 and 472, 1299, 1613 for Windows. tests: re-enable 2086, and 472, 1299, 1613 for Windows. Nov 26, 2024
@vszakats vszakats changed the title tests: re-enable 2086, and 472, 1299, 1613 for Windows. tests: re-enable 2086, and 472, 1299, 1613 for Windows Nov 26, 2024
@vszakats vszakats closed this in 96f7547 Nov 27, 2024
@vszakats vszakats deleted the t-undisable branch November 27, 2024 10:20
@MarcelRaad
Copy link
Member

Ah, test 2086 [Pre-request callback for HTTP IPv6] still fails for me:
https://curl.se/dev/log.cgi?id=20241128050932-2865694#prob1
https://curl.se/dev/log.cgi?id=20241128062717-2916001#prob1
Because of:

=== Start of file stderr2086
URL: [;1];60179\2086#ipv6

Looks like we have to tell MSYS2 to leave what it recognizes as path separators alone.

@vszakats
Copy link
Member Author

vszakats commented Dec 2, 2024

Ah, test 2086 [Pre-request callback for HTTP IPv6] still fails for me: https://curl.se/dev/log.cgi?id=20241128050932-2865694#prob1 https://curl.se/dev/log.cgi?id=20241128062717-2916001#prob1 Because of:

=== Start of file stderr2086
URL: [;1];60179\2086#ipv6

Looks like we have to tell MSYS2 to leave what it recognizes as path separators alone.

Isn't this a function of MSYS2 version? Its changelog says something changed around here recently:
https://www.msys2.org/news/#2024-11-03-disabling-mingw-w64-wildcard-support-by-default

If so, I still have no idea how to detect this and force it to work for older MSYS2 versions.

@MarcelRaad
Copy link
Member

I think we just need to set MSYS2_ARG_CONV_EXCL in <setenv> as done in other tests, which is unrelated to globbing IIUC. No idea yet what the difference between my machine and the CI builds is.

@vszakats
Copy link
Member Author

vszakats commented Dec 2, 2024

I think we just need to set MSYS2_ARG_CONV_EXCL in <setenv> as done in other tests, which is unrelated to globbing IIUC. No idea yet what the difference between my machine and the CI builds is.

Are you also using the freshest MSYS2?

edit: based on buildinfo.compiler.version: 10.2.0 via https://curl.se/dev/log.cgi?id=20241203050535-3961987, this may be an older MSYS2 without the recent change.

That envvar is suspect indeed, though I had very little experience/success with it,
and my only way to try is in CI.

We can certainly put it back to the DISABLE list if there is no obvious solution.

edit: a milder solution is to ignore this test via TFLAGS in the failing envs.

@MarcelRaad
Copy link
Member

Yes, it's indeed an older version - I'm a bit hesitant to update it as updates regularly break the e-mail client. I can confirm that setting MSYS2_ARG_CONV_EXCL as in other tests fixes the issue for me. Maybe newer versions just have a better path detection.
PR here: #15677

MarcelRaad added a commit to MarcelRaad/curl that referenced this pull request Dec 4, 2024
Older MSYS2 versions treat the URL as paths list and convert them from
UNIX to Windows format. There's no path here that needs to be
converted, so disable path conversion for this test as done for others.

Fixes curl#15644 (comment)
Closes curl#15677
MarcelRaad added a commit to MarcelRaad/curl that referenced this pull request Dec 8, 2024
Older MSYS2 versions treat the URL as paths list and convert them from
UNIX to Windows format. There's no path here that needs to be
converted, so disable path conversion for this test as done for others.

Fixes curl#15644 (comment)
Closes curl#15677
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration tests
Development

Successfully merging this pull request may close these issues.

2 participants