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: reformat error messages to avoid tripping MSBuild #16583

Closed
wants to merge 16 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Mar 6, 2025

Change the format of error messages sent to stderr from tests and test
servers. As a workaround to avoid triggering Visual Studio's MSBuild
tool's built-in regexp matcher, and making it mark builds failed for
reasons we don't want them to hard fail.

Roughly, the pattern to avoid is the word "error" (case-insensitive)
in the same line with a colon :.

It affected GHA/windows MSVC CI jobs, causing flakiness:

CUSTOMBUILD : fopen() failed with error : 13 Permission denied [D:\a\curl\curl\bld\tests\test-ci.vcxproj]
  Error opening file: log/4/smtp_sockfilt.log
[...]
CUSTOMBUILD : fopen() failed with error : 13 Permission denied [D:\a\curl\curl\bld\tests\test-ci.vcxproj]
  Error opening file: log/8/imap_sockfilt.log
  Msg not logged: 00:18:10.656000 > 178 bytes data, server => client
[...]
  TESTDONE: 1629 tests out of 1634 reported OK: 99%
  Building Custom Rule D:/a/curl/curl/tests/CMakeLists.txt
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\MSBuild\Microsoft\VC\v170\Microsoft.CppCommon.targets(254,5): error MSB8066: Custom build for 'D:\a\curl\curl\bld\CMakeFiles\621f80ddbb0fa48179f056ca77842ff0\test-ci.rule;D:\a\curl\curl\tests\CMakeLists.txt' exited with code -1. [D:\a\curl\curl\bld\tests\test-ci.vcxproj]
Error: Process completed with exit code 1.

Ref: https://github.com/curl/curl/actions/runs/13643149623/job/38137076210?pr=16490#step:14:3125
Ref: https://github.com/curl/curl/actions/runs/13688765792/job/38277961720?pr=16582#step:14:1717

The IgnoreStandardErrorWarningFormat="true" MSBuild Exec option
controls this behavior:
https://learn.microsoft.com/visualstudio/msbuild/exec-task#parameters
I couldn't figure out a way to apply it to CMake builds.

MSBuid pattern matching rules:
https://github.com/dotnet/msbuild/blob/353c0f3d37957cc98bfa6a76b568d70d12193fc3/src/Shared/CanonicalError.cs
https://learn.microsoft.com/visualstudio/msbuild/msbuild-diagnostic-format-for-tasks

Note: There may be further error messages output from runtests scripts,
that use this format, which are not explicitly fatal. They may need
future fixes.

Thanks-to: Dion Williams
Ref: #14854 (comment)
Ref: #14854 (reply in thread)


I could not figure out a way to tell MSBuild to stop regexp matching the log and auto-fail when seeing the word "error" in it. IMO this is an unexpected and annoying feature, esp. without ways to control this behavior. It reminds of the fight with PowerShell which did the same whenever seeing anything output to stderr.

  • for better or worse, mitigate by reformatting internal error message to avoid msbuild interfering with them.

@vszakats vszakats marked this pull request as draft March 6, 2025 03:14
@github-actions github-actions bot added tests CI Continuous Integration labels Mar 6, 2025
@testclutch

This comment was marked as resolved.

@vszakats vszakats marked this pull request as ready for review March 6, 2025 11:07
@vszakats vszakats changed the title GHA/windows: msbuild vs runtests tests: reformat error messages to avoid tripping MSBuild Mar 6, 2025
@vszakats vszakats added the Windows Windows-specific label Mar 6, 2025
@vszakats
Copy link
Member Author

vszakats commented Mar 6, 2025

MSBuild should allow to disable this feature. If there is a way,
I couldn't find it, via CMake at least.

So, all we have is this unfortunate workaround.

It doesn't cover all possible stderr error message, but should catch
those coming from actual tests or test servers. Its downside is that
it doesn't guarantee someome committing an error message in the
problematic format and reintroduce an issue anytime in the future.
I guess we may address this by using a 'logmsg' API call exclusively
and making sure that it transforms or doesn't allow the "flaky style"
in the first place.

Hm... or, perhaps we launch the test step manually, outside of CMake?

That's ugly but could work in CI. Outside CI it doesn't help when when
someone expects cmake --target test-ci to work without such issues.

Maybe for CI we can do it anyway, just to be 100%.

edit: More downsides:

  • it may use a different Perl copy than the one detected by CMake.
  • test-ci runtests invocation options need manual sync now with
    the GHA/windows workflow.

@vszakats vszakats changed the title tests: reformat error messages to avoid tripping MSBuild tests: reformat error messages to avoid tripping MSBuild, skip MSBuild in CI for tests Mar 6, 2025
@vszakats vszakats changed the title tests: reformat error messages to avoid tripping MSBuild, skip MSBuild in CI for tests tests: reformat error messages to avoid tripping MSBuild, skip MSBuild in CI Mar 6, 2025
@vszakats vszakats force-pushed the runtests-vs-msbuild2 branch from 0e89fd1 to b1b02f0 Compare March 6, 2025 15:42
@vszakats
Copy link
Member Author

vszakats commented Mar 6, 2025

The CI workaround doesn't work. It's also very impractical. Backtracking.

=== Start of file stderr1119
 Microsoft (R) C/C++ Optimizing Compiler Version 19.43.34808 for x64
 Copyright (C) Microsoft Corporation.  All rights reserved.
 curl.h
 D:\a\curl\curl\include\curl\system.h(313): fatal error C1083: Cannot open include file: 'inttypes.h': No such file or directory
 Error preprocessing ../../tests/../include/curl/curl.h at ../../tests/test1119.pl line 79.
=== Start of file stderr1167
 Microsoft (R) C/C++ Optimizing Compiler Version 19.43.34808 for x64
 Copyright (C) Microsoft Corporation.  All rights reserved.
 curl.h
 D:\a\curl\curl\include\curl\system.h(313): fatal error C1083: Cannot open include file: 'inttypes.h': No such file or directory
 Error preprocessing ../../tests/../include/curl/curl.h at ../../tests/test1167.pl line 118.

Without CMake, the VS environment is not setup, and setting it up correctly, in sync with CMake is also impractical.

@vszakats vszakats force-pushed the runtests-vs-msbuild2 branch from 1e8d6e1 to e2e0585 Compare March 6, 2025 16:18
@vszakats vszakats changed the title tests: reformat error messages to avoid tripping MSBuild, skip MSBuild in CI tests: reformat error messages to avoid tripping MSBuild Mar 6, 2025
@vszakats vszakats closed this in 9463769 Mar 6, 2025
@vszakats vszakats deleted the runtests-vs-msbuild2 branch March 6, 2025 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration tests Windows Windows-specific
Development

Successfully merging this pull request may close these issues.

2 participants