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

Remove spaces from test bundle identifiers #288

Merged
merged 1 commit into from
Jun 11, 2022

Conversation

franklinsch
Copy link
Contributor

Bug/issue #, if applicable: rdar://94811638

Summary

Removes spaces from the bundle identifiers using in Swift-DocC tests to work around a bug in macOS Ventura Beta 1 (94747378).

Dependencies

None.

Testing

N/A, not a user facing change.

Checklist

  • Added tests
  • Ran the ./bin/test script and it succeeded
  • Updated documentation if necessary

@franklinsch
Copy link
Contributor Author

@swift-ci please test

@ethan-kusters
Copy link
Contributor

Should we add an XCTSkip'd test somewhere that is testing this? So we know if this regresses again in the future? (Unclear to me if having spaces in a bundle ID is supported or not)

@franklinsch
Copy link
Contributor Author

That would be testing Foundation behavior here, which I'm not convinced we should do. According to the URL RFC, spaces are disallowed in URL hosts, which is where bundle identifiers are used.

@ethan-kusters
Copy link
Contributor

That would be testing Foundation behavior here, which I'm not convinced we should do. According to the URL RFC, spaces are disallowed in URL hosts, which is where bundle identifiers are used.

Maybe we should be transforming bundle IDs that contain spaces then and replacing them with hyphens? Can definitely be a future PR.

But since this is allowed behavior currently it seems like we're just hiding future issues by removing these tests entirely.

@franklinsch
Copy link
Contributor Author

Given the URL RFC, I think we should actually treat bundle identifiers with spaces as invalid. The bug here is that DocC currently accepts them but shouldn't. I filed #290

Removes spaces from the bundle identifiers using in Swift-DocC tests to
work around a bug in macOS Ventura (94747378).

rdar://94811638
@franklinsch
Copy link
Contributor Author

@swift-ci please test

@franklinsch franklinsch merged commit a5b46ab into swiftlang:main Jun 11, 2022
@franklinsch franklinsch deleted the test-bundle-id-spaces branch June 11, 2022 13:18
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.

2 participants