Skip to content

GHA/windows: run tests in a WinXP build#17426

Closed
vszakats wants to merge 2 commits intocurl:masterfrom
vszakats:gha-xp
Closed

GHA/windows: run tests in a WinXP build#17426
vszakats wants to merge 2 commits intocurl:masterfrom
vszakats:gha-xp

Conversation

@vszakats
Copy link
Member

No description provided.

@vszakats vszakats added tests Windows Windows-specific CI Continuous Integration labels May 23, 2025
@vszakats
Copy link
Member Author

vszakats commented May 23, 2025

There is a surprise here: Tests did run successfully, but very slowly.
Including test 3026, but a bunch of others too:

test 2048...[pinnedpubkey no-match must fail even when insecure]
-------e--- OK (1486 out of 1577, remaining: 00:10, took 35.241s, duration: 02:55)
test 3029...[HTTP with multiple -D]
---d--oe--- OK (1557 out of 1577, remaining: 00:02, took 78.204s, duration: 03:44)
test 3030...[HTTP with multiple transfers in one -D]
---d--oe--- OK (1558 out of 1577, remaining: 00:03, took 161.099s, duration: 05:10)
test 3024...[HTTPS GET to localhost, last subject alt name matches, CN does not match (Schannel variant)]
--pd---e--- OK (1552 out of 1577, remaining: 00:05, took 206.315s, duration: 05:52)
test 3028...[HTTP GET when PROXY Protocol enabled behind a proxy]
--p--P-e--- OK (1556 out of 1577, remaining: 00:04, took 222.164s, duration: 06:08)
test 3031...[--output-dir with --create-dirs]
--p---oe--- OK (1559 out of 1577, remaining: 00:04, took 254.483s, duration: 07:10)
test 3032...[HTTP redirect loop 3x swsbounce test]
--pd---e--- OK (1560 out of 1577, remaining: 00:05, took 270.187s, duration: 08:15)
test 3100...[RTSP Authentication check]
--pd---e--- OK (1561 out of 1577, remaining: 00:05, took 236.172s, duration: 09:07)
test 3101...[HTTP auth without redirection protocols]
--pd---e--- OK (1562 out of 1577, remaining: 00:05, took 197.396s, duration: 09:10)
test 3026...[curl_global_init thread-safety]
-------e--- OK (1554 out of 1577, remaining: 00:08, took 404.906s, duration: 09:11)
test 3103...[CURLOPT_COOKIELIST without expiry]
--pd---e--- OK (1564 out of 1577, remaining: 00:04, took 121.360s, duration: 09:11)
test 3104...[CURLOPT_COOKIELIST with Netscape format]
--pd---e--- OK (1565 out of 1577, remaining: 00:04, took 56.498s, duration: 09:11)
test 3105...[curl_multi_remove_handle twice]
[...]
est 3027...[Get a file via FTP but 550 after MDTM command]
--pd---e--- OK (1555 out of 1577, remaining: 00:07, took 408.999s, duration: 09:15)
test 3102...[verify certificate chain order with simple HTTPS GET]
--p----e--- OK (1563 out of 1577, remaining: 00:04, took 186.970s, duration: 09:15)
test 3023...[HTTPS GET to localhost, first subject alt name matches, CN does not match (Schannel variant)]
--pd---e--- OK (1551 out of 1577, remaining: 00:09, took 409.476s, duration: 09:15)

https://github.com/curl/curl/actions/runs/15208036424/job/42775602819?pr=17426#step:15:3339

  -time-  test
  ------  ----
  409.176  3023
  408.592  3027
  186.783  3102
  169.735  3100
  143.028  3101
  119.094  3032
  112.289  3103
  108.076  3031
  83.590  3028
  54.739  3104
  43.830  3030
  29.192  3024
  09.842  3029

(there seems to be discrepancy between the time measurement summary and the
test run log lines?)

Suspicious that the slow 3026 tests observed in #9412 may not be old mingw
related, but perhaps related to the fact that old mingw targets XP (or older) by
default, and modern mingw, MSVC all target newer versions by default. This is
the first PR in curl that forces a WinXP build with a newer compiler and runs
tests on it.

[edit: a subsequent test confirmed that performance is unchanged with or
without the patch implemented in #9412. Suggesting the hypotesis above does
not hold.]

One thing to test how re-applying #17414 would change the results. Let's see.

@vszakats
Copy link
Member Author

vszakats commented May 23, 2025

Another run of this patch as applied onto #17413 (that drops one of the two dynamic DLL loads for XP builds),
producing the same slowness:
https://github.com/curl/curl/actions/runs/15208050776/job/42775657992?pr=17413

  -time-  test
  ------  ----
  402.631  3023
  402.608  3024
  402.416  3027
  126.559  3102
  111.703  3032
  97.131  3030
  84.835  3031
  82.756  3101
  73.910  3028
  71.095  3029
  43.219  3103
  28.346  3104
  13.693  1470
  11.083  0312

@vszakats vszakats marked this pull request as draft May 23, 2025 11:09
@vszakats
Copy link
Member Author

vszakats commented May 23, 2025

Same results with the old mingw perf mitigation drop (#17414) reverted, including test 3026:

test 3026...[curl_global_init thread-safety]
-------e--- OK (1554 out of 1577, remaining: 00:08, took 432.126s, duration: 09:42)
[...]
  -time-  test
  ------  ----
  436.497  3024
  436.285  3027
  238.112  3028
  152.561  3032
  125.164  3031
  114.238  3100
  111.422  3101
  100.265  3030
  93.904  3029
  71.190  3102
  28.433  3103
  25.765  3104
  11.703  0312
  10.135  1470

https://github.com/curl/curl/actions/runs/15208745450/job/42777799811?pr=17426#step:15:3446

It feels kind of a waste of time to pursue the root cause. I'd instead be opting
to not run tests for WinXP builds in CI. It does work, but slowly and it's not worth
the pain for each run. It also doesn't fit into the 10-minute budget allocated for
the test step.

If someone is willing to dig deeper, this is something we can revisit.

@testclutch
Copy link

Analysis of PR #17426 at a5e22517:

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

Generated by Testclutch

@bagder
Copy link
Member

bagder commented May 25, 2025

I'd instead be opting to not run tests for WinXP builds in CI

Absolutely - they help exactly no one. And the second the builds cause problems we can drop them rather than wasting time fixing them...

@vszakats vszakats closed this May 25, 2025
@vszakats vszakats deleted the gha-xp branch May 25, 2025 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Continuous Integration on-hold tests Windows Windows-specific

Development

Successfully merging this pull request may close these issues.

3 participants