Skip to content

Fix silent test failures on windows #68676

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

Closed
wants to merge 1 commit into from
Closed

Fix silent test failures on windows #68676

wants to merge 1 commit into from

Conversation

tristanlabelle
Copy link
Contributor

The build was succeeding after test failures because exit /b from within a call only exits the subroutine, not the full script:

IF "%SKIP_TEST%"=="0" call :TestSwift
cmake --build %BuildRoot%\1 --target check-swift || (exit /b)

We need to also bubble the error after the call returns.

@tristanlabelle tristanlabelle requested a review from a team as a code owner September 21, 2023 19:24
@bnbarham
Copy link
Contributor

@swift-ci please test

@bnbarham
Copy link
Contributor

Is this also happening on main then 🤔?

@bnbarham
Copy link
Contributor

swift-ci@EC2AMAZ-JCVOGM7 C:\Users\swift-ci\jenkins\workspace\swift-PR-windows>move T:\package\toolchain\toolchain.msi T:\package   || (exit /b ) 
The system cannot find the file specified.

@bnbarham
Copy link
Contributor

I've included in #68692. @tristanlabelle could you put this up on main 🙏?

@compnerd
Copy link
Member

@bnbarham the MSI restructuring on main would explain why toolchain.msi is not found on main.

@compnerd
Copy link
Member

@swift-ci please build toolchain Windows platform

@bnbarham
Copy link
Contributor

I've cherry-picked this to the other 5.9 branch anyway @compnerd. Can close this one.

@tristanlabelle
Copy link
Contributor Author

@bnbarham main already has this, albeit in a slightly different form:

image

I will close this in favor of #68692, and I can help look into the missing msi.

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

Successfully merging this pull request may close these issues.

3 participants