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

[scripts/ci] CI Test Modified Ports does not fail as expected #40191

Conversation

WangWeiLin-MV
Copy link
Contributor

@WangWeiLin-MV WangWeiLin-MV commented Jul 31, 2024

Fix #40124.

Check the issue link, vcpkg ci reported REGRESSION: cyrus-sasl:x64-windows failed with BUILD_FAILED. but the CI task does not emit error.

Fix it by moving step Test Modified Ports from AzureCLI@2 to PowerShell@2.

@WangWeiLin-MV WangWeiLin-MV added info:internal This PR or Issue was filed by the vcpkg team. category:infrastructure Pertaining to the CI/Testing infrastrucutre labels Jul 31, 2024
@WangWeiLin-MV WangWeiLin-MV force-pushed the scripts/ci/fail-on-stderr branch 2 times, most recently from d3cfbbe to 67e6591 Compare July 31, 2024 09:16
@WangWeiLin-MV WangWeiLin-MV changed the title [test] ci failOnStandardError [test] CI Test Modified Ports does not fail as expected Jul 31, 2024
@WangWeiLin-MV WangWeiLin-MV changed the title [test] CI Test Modified Ports does not fail as expected [scripts/ci] CI Test Modified Ports does not fail as expected Jul 31, 2024
@WangWeiLin-MV WangWeiLin-MV force-pushed the scripts/ci/fail-on-stderr branch 5 times, most recently from 777b1b6 to 52511e6 Compare July 31, 2024 11:16
@WangWeiLin-MV WangWeiLin-MV force-pushed the scripts/ci/fail-on-stderr branch 2 times, most recently from a61befd to 8f83d6d Compare August 1, 2024 02:22
@WangWeiLin-MV
Copy link
Contributor Author

WangWeiLin-MV commented Aug 1, 2024

See fixes in #40198


Information

An earliest abnormal result found so far is Pipelines - Run 20240716.15 logs (visualstudio.com)

A recent normal result failed as expected is Pipelines - Run 20240716.11 logs (visualstudio.com)

According to the test Pipelines - Run 20240731.15 logs (visualstudio.com), all Windows jobs are not failed as expect.

According to the test Pipelines - Run 20240731.20 logs (visualstudio.com), it fails as expected with PowerShell@2.

Possible regression #39896 #39952

Reference

PowerShell@2 - failOnStderr
AzureCLI@2 - failOnStandardError

Test

The CI task test pass with Windows jobs:

@BillyONeal
Copy link
Member

Yeah this is squarely my fault in #39952 -- I saw failures for macOS and Linux and assumed 'surely identical code will have identical behavior everywhere'. I don't understand exactly why failOnStandardError is not working only on Windows, or why my attempts to make more reduced repros to file against the folks who own AzureCLI@2 have not worked.

Either way, propagating the exit code rather than relying on stderr being special is just a good idea in the first place, which shuts this down completely. I've submitted that as an alternative fix in #40198

Thanks for pointing this out!

@BillyONeal
Copy link
Member

Thanks again for the analysis! I believe this problem is now fixed.

@BillyONeal BillyONeal closed this Aug 1, 2024
@WangWeiLin-MV WangWeiLin-MV deleted the scripts/ci/fail-on-stderr branch August 2, 2024 01:56
@WangWeiLin-MV WangWeiLin-MV restored the scripts/ci/fail-on-stderr branch August 13, 2024 07:31
@WangWeiLin-MV WangWeiLin-MV reopened this Aug 13, 2024
@WangWeiLin-MV
Copy link
Contributor Author

WangWeiLin-MV commented Aug 13, 2024

Testing

  • Test PowerShell@2 failOnStderr
  • Test AzureCLI@2 failOnStandardError

Reproduced

#include <windows.h>
#include <iostream>
int main() {
    std::cout << "stdout" << std::endl;
    HANDLE hStderr = ::GetStdHandle(STD_ERROR_HANDLE);
    if (hStderr == INVALID_HANDLE_VALUE) {
      return 1;
    }
    DWORD written = 0;
    const char* msg = "stderr\n";
    if (!::WriteFile(hStderr, msg, strlen(msg), &written, nullptr)) {
        return 1;
    }
    return 0;
}

Reproduced with ::WriteFile, see vcpkg-tool/src/vcpkg/base/messages.cpp#L307-L353.

See microsoft/azure-pipelines-tasks#20291

@WangWeiLin-MV WangWeiLin-MV force-pushed the scripts/ci/fail-on-stderr branch 4 times, most recently from e199c41 to 4a378f9 Compare August 13, 2024 10:06
@WangWeiLin-MV WangWeiLin-MV force-pushed the scripts/ci/fail-on-stderr branch 16 times, most recently from 92940fc to edb8180 Compare August 14, 2024 08:44
@WangWeiLin-MV WangWeiLin-MV deleted the scripts/ci/fail-on-stderr branch August 15, 2024 02:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:infrastructure Pertaining to the CI/Testing infrastrucutre info:internal This PR or Issue was filed by the vcpkg team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hidden CI failures
3 participants