Miscellaneous test changes to avoid making assumptions about what's a valid URL #586
+40
−55
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
(This is verifying Foundation behavior that's not redirect specific)
(In practice the port is validated before this but some tests skip that)
franklinsch
approved these changes
May 16, 2023
|
@swift-ci please test |
|
@swift-ci please test |
# Conflicts: # Package.resolved
|
@swift-ci please test |
| @@ -54,7 +54,7 @@ class ResolvedTopicReferenceTests: XCTestCase { | |||
| do { | |||
| let resolvedOriginal = ResolvedTopicReference(bundleIdentifier: "bundleID", path: "/path/sub-path", fragment: "fragment", sourceLanguage: .swift) | |||
|
|
|||
| let unresolved = UnresolvedTopicReference(topicURL: ValidatedURL(parsingExact: "doc://host.name/;;;")!) | |||
| let unresolved = UnresolvedTopicReference(topicURL: ValidatedURL(parsingExact: "doc://host.name/ ")!) | |||
There was a problem hiding this comment.
Test Suite 'Selected tests' started at 2023-07-25 18:53:24.131
Test Suite 'SwiftDocCPackageTests.xctest' started at 2023-07-25 18:53:24.132
Test Suite 'ResolvedTopicReferenceTests' started at 2023-07-25 18:53:24.132
Test Case '-[SwiftDocCTests.ResolvedTopicReferenceTests testAppendingReferenceWithEmptyPath]' started.
SwiftDocCTests/ResolvedTopicReferenceTests.swift:57: Fatal error: Unexpectedly found nil while unwrapping an Optional value
Looks like this change made this URL initializer fail in CI.
|
@swift-ci please test |
|
@swift-ci please test |
|
@swift-ci please test |
d-ronnqvist
added a commit
to d-ronnqvist/swift-docc
that referenced
this pull request
Jul 26, 2023
… valid URL (swiftlang#586) * Avoid assumption about order of encoded JSON in test * Remove test that verifies what characters redirect path support (This is verifying Foundation behavior that's not redirect specific) * Simplify test that only verify validation of one URL string * Require that source repository base URLs include scheme and host * Avoid testing curation of headings (this isn't supported) * Update test URL to use characters that DocC will replace with "-" * Only print the preview URL if it's non-nil (In practice the port is validated before this but some tests skip that) * Update Swift-Markdown to pick up a index-out-of-bounds fix * Update test URL to use other non-path-allows characters * Revert "Update test URL to use characters that DocC will replace with "-"" 7d1f3bd rdar://108237260
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Bug/issue #, if applicable: rdar://108237260
Summary
This makes a handful of changes, mainly in tests, to avoid making assumptions about what's a valid URL. Each change is its own commit that describe the reason for that change.
I think that's the type of values we expect for this configuration. If we don't have any specific validation for these values I think we should remove those test assertions (and possibly use a string instead of a URL)
Dependencies
None
Testing
Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/testscript and it succeeded