Skip to content

Tests: Fix 22 warning in BuildCommandTests #8509

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

Merged
merged 1 commit into from
Apr 17, 2025
Merged

Conversation

pmattos
Copy link
Contributor

@pmattos pmattos commented Apr 15, 2025

Motivation:

Reduce noise from solvable warnings.

Modifications:

We should throw XCTSkip("...") but try XCTSkipIf(true, "..."). The BuildCommandTests was mixing both (i.e., try XCTSkip("...")), making the test a nop actually.

Result:

Less 22 warnings I guess -:)

@pmattos pmattos changed the title Fix 22 warning in BuildCommandTests Tests: Fix 22 warning in BuildCommandTests Apr 15, 2025
@plemarquand
Copy link
Contributor

@swift-ci please test

@pmattos
Copy link
Contributor Author

pmattos commented Apr 16, 2025

@swift-ci test macos

@pmattos
Copy link
Contributor Author

pmattos commented Apr 16, 2025

@swift-ci test self hosted windows

@pmattos
Copy link
Contributor Author

pmattos commented Apr 17, 2025

@swift-ci test macOS

@pmattos pmattos enabled auto-merge (squash) April 17, 2025 01:24
@pmattos
Copy link
Contributor Author

pmattos commented Apr 17, 2025

@swift-ci test macos

1 similar comment
@pmattos
Copy link
Contributor Author

pmattos commented Apr 17, 2025

@swift-ci test macos

@pmattos pmattos merged commit 2c4c785 into main Apr 17, 2025
7 checks passed
@pmattos pmattos deleted the pmattos/fix-tests-warnings branch April 17, 2025 08:54
johnbute pushed a commit to johnbute/fork-swift-package-manager that referenced this pull request Apr 17, 2025
### Motivation:

Reduce noise from solvable warnings.

### Modifications:

We should `throw XCTSkip("...")` but `try XCTSkipIf(true, "...")`.
The BuildCommandTests was mixing both (i.e., `try XCTSkip("...")`), 
making the test a nop actually.
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